Elegancki kod., Kod który działa ale jest "brzydki" Opcje

[saund]

Hej!

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

[HTML] pobierz, plaintext 
function maximumZindex () {
	"use strict";
	var highestZindex = 0;
	frame.contents().find('body *').each(
		function zIndexCheck () {
			if (isNaN(Number($(this).css("zIndex")))) ; // Do nothing when .css() return "auto"
			else if ($(this).css("zIndex")>highestZindex) 
				highestZindex = $(this).css("zIndex");
		})
	highestZindex++;
	return highestZindex;
};
//v2 
function maximumZindex () {
	"use strict";
	var highestZindex = 0;
	frame.contents().find('body *').each(
		function zIndexCheck () {
			if (!isNaN(Number($(this).css("zIndex")))) {
				if($(this).css("zIndex")>highestZindex)
				highestZindex = $(this).css("zIndex");
			};
		})
	highestZindex++;
	return highestZindex;
};
 
[HTML] pobierz, plaintext 

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" ?

Ten post edytował saund 18.08.2013, 22:43:29

[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 
function maximumZindex () 
{
	"use strict";
	var highestZindex = 0;
        var allElements = frame.contents().find('body *');
 
	allElements.each(
		function  () {
                        var currentZIndex = $(this).css("zIndex");
			if (!isNaN(Number(currentZIndex))) {
				if(currentZIndex >highestZindex)
				highestZindex = currentZIndex;
			};
                        highestZindex++;
		})
 
        return highestZindex;
};
[JAVASCRIPT] pobierz, plaintext 

Ten post edytował lukasz1985 2.09.2013, 14:12:17