Pomoc - Szukaj - Użytkownicy - Kalendarz
Pełna wersja: Elegancki kod.
Forum PHP.pl > Forum > Po stronie przeglądarki > JavaScript
saund
Hej!

Jestem początkującym programistą JS i korzystam z framewoka jQuery.

  1. function maximumZindex () {
  2. "use strict";
  3. var highestZindex = 0;
  4. frame.contents().find('body *').each(
  5. function zIndexCheck () {
  6. if (isNaN(Number($(this).css("zIndex")))) ; // Do nothing when .css() return "auto"
  7. else if ($(this).css("zIndex")>highestZindex)
  8. highestZindex = $(this).css("zIndex");
  9. })
  10. highestZindex++;
  11. return highestZindex;
  12. };
  13. //v2
  14. function maximumZindex () {
  15. "use strict";
  16. var highestZindex = 0;
  17. frame.contents().find('body *').each(
  18. function zIndexCheck () {
  19. if (!isNaN(Number($(this).css("zIndex")))) {
  20. if($(this).css("zIndex")>highestZindex)
  21. highestZindex = $(this).css("zIndex");
  22. };
  23. })
  24. highestZindex++;
  25. return highestZindex;
  26. };
  27.  


Są dwie wersje tego samego skryptu.
Opis działania: Skrypt wyszukuję największy z-index występujący na stronie w ramce iframe ( $("#frame") = frame ). Pierwszy IF sprawdza czy .css() nie zwraca wartości auto (Nie chcę porównywać "auto" z liczbą a parsowanie "auto" na inta wzraca NaN)jeśli tak to nic nie robi jeśli nie to porównuje do ostatniego największego z-index jakiego napotkał i jeśli jest większy to staje się aktualnym największym z indexem.

Postanowiłem zapytać się znajomego, który jest programistą JS czy kod jest poprawny (pokazałem mu pierwszą wersje) powiedział, że pusty IF nie przejdzie nigdzie więc ok napisałem drugą i powiedział, że kod jest beznadziejny i oprócz tego, że brakuję wywołania wewnętrznej funkcji zIndexCheck() wiem bo tylko tego się od niego dowiedziałem. Zasugerował mi też, że $('body *').each(...) jest za wolny w jaki inny sposób(szybszy) mogę sprawdzić jakie z-index mają wszystkie objekty występujące na stronie.

Co mogę zrobić, żeby ten kod był "elegancki" ?
sunpietro
Po pierwsze, stosuj jednolity standard formatowania kodu JS.
Po drugie, jeśli chcesz iterować po wszystkich elementach DOM, to lepiej do tego użyć czystego JS:
Kod
var items = iframe.getElementsByTagName("*");
for (var i = items.length; i > 0;  i--;) {
    sprawdzanie
}
zegarek84
Cytat(sunpietro @ 19.08.2013, 06:58:19 ) *
Po drugie, jeśli chcesz iterować po wszystkich elementach DOM, to lepiej do tego użyć czystego JS:
Kod
var items = iframe.getElementsByTagName("*");
for (var i = items.length; i > 0;  i--;) {
    sprawdzanie
}

sorki, bez urazy, ale nie do końca jest to prawda i masz jeszcze błąd w kodzie... -> powinieneś zrozumieć (w jQ jest to zaimpementowane) http://stackoverflow.com/questions/926916/...e-in-javascript

po za tym nie pamiętam, jak dokładnie jest pisany silnik Sizzle ale tam jest to w miare optymalnie napisane i do prostych przypadków o ile dostępna metoda to korzysta z szybkiego .querySelectorAll, a jak nie dostępne to zapytania CSS są optymalnie przeszukiwane - NAJWIĘKSZY PROBLEM KORZYSTAJĄCYCH Z jQ jest brak "buforowania" referencji do elementów które wielokrotnie wyszukują...

wracając do tematu czemu zwiększasz o 1 highestZindex, po za tym z samym zIndex o ile pamiętam to nie maiłbyś tak łatwo jak to się wydaje po samej wartości ;p, o ile pamiętam jeśli rodzice i inne elementy mają nadane zIndex to w sumie masz wartości pośrednie a nie po cyfrze ;p... jeśli nie nadasz zIndex to w przeglądarce elementy na tym samym poziomie i tak mają jakby nadany domyślny zIndex którego się nie odczytuje w kolejności występowania (raz musiałem odwracać elementy do jednego tricku ;p)

co do komentarza tamtego "programisty" to się czepiał szczegółów... ale po części miał rację gdyż po co pisać pierwszy warunek gdzie nic nie robisz?? ale stosuje się takie ify w trochę innym wykonaniu, w pętli do break i w grubszej funkcji przy odwróconych warunkach dla czytelności kodu do przerwania dalszego wykonywania kodu przez if(true) return wartosc_lub_nic;

z tym "za wolny" już napisałem co i jak, fakt masz lekki narzut na szybkości ale nie za wielki... jeśli chcesz zyskać na szybkości to popraw przykład podany przez @sunpietro (pętla poprawna choć iteracja od tyłu - choć iteracja od tyłu fajnie przydaje się do super szybkiego usuwania elementów z listy ;p), a gdzie drobny błąd podałem w linku wyżej...

nigdy nie programowałem zawodowo jednak 3 lata temu współpracowałem z manifo przy tworzeniu edytora WWW (swoją drogą zastanawiam się, czemu tego wpisu jeszcze nie zaktualizowali)
http://pl.manifo.com/o_nas.html - Grzegorz Nowak, zajmowałem się głównie JS, w tym edytorem w iframe na bazie modyfikacji CLEditor'a, różnicami przeglądarkowymi
lukasz1985
Nie nadawałbym nazwy funkcjom anonimowym, wskazuje na to logika i dodatkowo problemy z IE, gdzie ta funkcja może wyciec do zakresu globalnego. Funkcje anonimowe to fragmenty kodu - używane są jako zwykłe instrukcje, więc ich nazywanie jest niespójne z resztą kodu (nie nadajesz nazwy pętli for, while, czy instrukcji if albo switch - poza przypadkiem używania zakładek ale to rzadki przypadek).

Poza tym raczej unikam wstawiania funkcji anonimowych, jeśli nie jest to trywialna funkcja, wtedy definiuję funkcję na zewnątrz zakresu i odwołuję się do jej referencji. Twoja "anonimowa" funkcja zIndexCheck jest trywialna, więc ok.

Jeśli funkcja jest bardziej rozbudowana - to raczej powinna być zaimplementowana w obiekcie - w tym przypadku - przydaje się funkcja "bind" obiektu "Function". Jeśli znajduje się w globalnej przestrzeni nazw, to staje się zbędne ale kod, który nie jest obiektowy może sprawiać problemy.

O wydajności nie mam dużo do powiedzenia - jedyne na co mogę zwrócić uwagę, to tworzenie łańcuchów wywołań. Więcej zmiennych powinno być na stosie, zamiast zmuszać komputer do ciągłego wywoływania metod pobierających. Chodzi o to, że zamiast pisać x razy "cosTam($(this).css("zIndex"))" czy "$(this).css("zIndex") > cosTam" - należałoby przechwycić wartość "$(this).css("zIndex")" do zmiennej i potem jej używać, zamiast za każdym razem ją pobierać. Kod staje się czytelniejszy i bardziej wydajny.

Generalnie napisałbym Twój kod tak:

[JAVASCRIPT] pobierz, plaintext
  1. function maximumZindex ()
  2. {
  3. "use strict";
  4. var highestZindex = 0;
  5. var allElements = frame.contents().find('body *');
  6.  
  7. allElements.each(
  8. function () {
  9. var currentZIndex = $(this).css("zIndex");
  10. if (!isNaN(Number(currentZIndex))) {
  11. if(currentZIndex >highestZindex)
  12. highestZindex = currentZIndex;
  13. };
  14. highestZindex++;
  15. })
  16.  
  17. return highestZindex;
  18. };
[JAVASCRIPT] pobierz, plaintext
To jest wersja lo-fi głównej zawartości. Aby zobaczyć pełną wersję z większą zawartością, obrazkami i formatowaniem proszę kliknij tutaj.
Invision Power Board © 2001-2025 Invision Power Services, Inc.