Mam nadzieję, że nie popełniłem poprzednich "złych" praktyk ;)
Niestety, popełniłeś - nawet te o których wspominaliśmy z @niezalogowany w Twoim poprzednim temacie.
- 6 krotnie korzystasz z pętli na tej samej zmiennej
for(var y=0;y<yNumb;y++)
7 krotnie loopujesz po innej zmiennej
for(var x = 0;x<xNumb;x++)
Mógłbyś zrobić sobie funkcję, w której miałbyś pętle for a warunek jej zakończenia (zmienne xNumber i yNumber) przekazywałbyś jako parametr.
Przykład powtarzalności (od linijki 76):
//Zmienianie stanu tabeli stateTable:
for(var y=0;y<yNumb;y++)
{
for(var x=0;x<xNumb;x++)
{
stateTable[x][y]=futureStateTable[x][y];
}
}
refreshCells();
i w następnej funkcji robisz prawie to samo (linijka 89):
function clearCells()
{
for(var y=0;y<yNumb;y++)
{
for(var x=0;x<xNumb;x++)
{
stateTable[x][y]=0;
}
}
refreshCells();
}
Dlaczego w 76 linijce nie wywołasz sobie funkcji clearCells() z przekazaniem parametru, którym byś sobie umieścił w stateTable[x[y] zero albo jakąś zmienną (np. tą futureStateTable[x][y])?
Przeszukałem sobie ten skrypt, i praktycznie w żadnej własnej funkcji nie przekazujesz parametru - a przecież dzięki temu możesz sobie ułatwić życie i skrócić kod. O tym wspominałem Tobie właśnie w poprzednim wątku. Cytując:
Widzę natomiast, że w kodzie masz trzy razy tą samą pętlę, a dodatkowo funkcje draw() i win( who ) wykonują prawie identyczny kod. Dlatego proponuję obie te funkcje złączyć w jedną i sterować środkiem przez jakiś dodatkowy parametr, albo wewnątrz tych funkcji robić odniesienie do kolejnej funkcji, która będzie zawierać pętlę i tam sterować parametrami. Właśnie to jest reużywalność i do tego służą funkcję - aby kod o zbliżonym działaniu móc wykonywać wielokrotną ilość razy, sterując jakimiś różnymi zachowaniami przez parametry.
- troszkę brak konsekwencji:
//Key listener
window.onkeypress = function(e)
{
var key = e.keyCode ? e.keyCode : e.which;
if (key == 32)
{
evolution();
}
}
Dlaczego to nie jest EventListener? Poniżej już dobrze robisz:
//Eventy
document.getElementById("click").addEventListener("mousedown", evolution);
document.getElementById("clear").addEventListener("mousedown", clearCells);
document.getElementById("random").addEventListener("mousedown", fillRandom);
document.getElementById("start").addEventListener('click', simulationStart);
document.getElementById("stop").addEventListener('click', simulationStop);
document.getElementById("speed").addEventListener("input", refreshSpeed);
- zapomniałeś o stosowaniu potrójnego operatora porównania
- dlaczego jedne zmienne deklarujesz poniżej funkcji, w której z nich korzystasz (np. wspomniane xNumb oraz yNumb - używasz już w pierwszej funkcji ,a deklarujesz w połowie skryptu), a inne (prawidłowo) deklarujesz przed użyciem w funkcji (np. simulation oraz speed)?
- przy kilkunastokrotnym odwoływaniu się do obiektu document pokusiłbym się o przekazanie go do IIFE jako parametr, aby JS nie musiał każdorazowo wyłazić poza IIFE i szukać tego pola w obiekcie window. Dla przypomnienia: odwołując się do jakiejś zmiennej, JS szuka jej deklaracji najpierw w lokalnym scope, a potem coraz wyżej, aż do scope'u globalnego (czyli obiektu window). Jeśli przekażesz sobie daną zmienną jako parametr, to JS nie musi wyłazić z lokalnego scope'u = kod wykonuje się szybciej.