[CODE REVIEW] Gra w kółko i krzyżyk. Początki javascript.

pytanie zadane 28 września 2016 w JavaScript przez Codeboy Stary wyjadacz (12,120 p.)
edycja 19 października 2016 przez Codeboy

Zacząłem uczyć się javascriptu i postanowiłem zrobić grę kółko i krzyżyk żeby nie była to sucha nauka.

Proszę o ocenę i wytkniecie wszelkich błędów ;) Co powinienem ulepszyć, czego się douczyć. Dopiero zaczynam z javascriptem wiec będę wdzięczny za wszystkie wskazówki. Najbardziej boli mnie sprawdzanie czy divy w linii są takie same, bo wygląda to co najmniej obszernie(no ale to pierwsze na co wpadłem) ;) Napisałem to w jeden wieczór, także jestem pewien ze jest sporo do poprawy. Dziękuje za poświęcony czas.

W miedzy czasie przejrzę inne tematy z code-review tej gry na forum ;).

https://github.com/NoMoExcuses/Kolko-i-krzyzyk

http://www.kolko-i-krzyzyk.awsome.pl/

1 odpowiedź

odpowiedź 28 września 2016 przez ScriptyChris Mędrzec (190,190 p.)
wybrane 30 września 2016 przez Codeboy

Najlepsza

Co do JavaScriptu:

całość skryptu zamknij w IIFE (chyba, że przepiszesz na ES6, to w bloku) => http://benalman.com/news/2010/11/immediately-invoked-function-expression/ oraz http://exploringjs.com/es6/ch_core-features.html#_from-iifes-to-blocks
skrypt z <head> przenieś na koniec <body> i wtedy możesz zrezygnować z window.onload
w pętlach stosujesz zmienne globalne (nie dodając słowka var, gdy dana nazwa zmiennej nie została wcześniej zadeklarowana, JavaScript domyślnie tworzy daną zmienną w obiekcie window - nie jest to wskazane, zwłaszcza jeśli nie jesteś tego świadom) - nie rób tego. W pętlach typu for stosuj albo var albo const / let => https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Statements/var
zapisy typu:
```
pola = pola +'<div id="'+i+'" class="pole" style="float: left" onclick="postaw('+i+')"><p>'+i+'</p></div>';
```
gdy tylko możesz nie styluj HTMLa w formie inline lecz nadawaj klasy i styluj w CSS => http://www.standardista.com/css3/css-specificity/ dodatkowo http://forum.pasja-informatyki.pl/109776/html-class-czy-id?show=109776#q109776
Nie dawaj w HTMLu atrybutów odpowiedzialnych za obsługę eventów typu onclick / onmouseover - do tego służy JavaScriptowy addEventListener => https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/addEventListener
```
document.getElementById(nr).setAttribute("class", "off");
```
Jeśli dodajesz klasę, to możesz to zrobić za pomocą element.classList.add('nazwa_klasy') i usuwanie poprzez element.classList.remove('nazwa_klasy') => https://developer.mozilla.org/en-US/docs/Web/API/Element/classList
stosuj potrójny operator porównania === / !== => https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Operators/Comparison_Operators
do tego polecam włączyć strict mode

2	komentarz 29 września 2016 przez niezalogowany całość skryptu zamknij w IIFE (chyba, że przepiszesz na ES6, to w bloku) Tutaj warto zauważyć, że domknięcie w bloku nie tworzy nowego scopa dla zmiennych zadeklarowanych za pomocą var. ES6 -> no-var (jeśli nie ma takiej potrzeby)

komentarz 29 września 2016 przez Codeboy Stary wyjadacz (12,120 p.)
przywrócone 6 października 2016 przez Codeboy

Wow, nie spodziewałem się takiej rozbudowanej odpowiedzi. Od razu zabieram się za lekturę i poprawę kodu ;). Dziękuje!

komentarz 29 września 2016 przez Codeboy Stary wyjadacz (12,120 p.)

Wstępnie, szybko zauważyłem że to co jest zawarte w filmach JS pana Miroslawa Zelenta rożni się od tego co miałbym teraz zaimplementować po uzyskanych radach. Do tej pory głownie z tych filmów czerpałem wiedzę i na tej podstawie zbudowałem ta grę. No i jest mała rozbieżność a filmy nie są jakieś stare, wiesz może z czego to wynika?

(np. zmienne w pętlach tez są bez var, skrypt jest w head itp.)

2	komentarz 29 września 2016 przez niezalogowany Bierz popcorn -> http://forum.pasja-informatyki.pl/122259/co-jest-nie-tak-z-najnowszym-odcinkiem-kursu-miroslawa-zelenta ; )

komentarz 30 września 2016 przez Codeboy Stary wyjadacz (12,120 p.)

Dzięki, trochę tego jest :D

komentarz 30 września 2016 przez Codeboy Stary wyjadacz (12,120 p.)

Przeczytałem wszystko(odnośnie poprawności kursów). Trochę mi to zajęło i przyznam ze trochę mną zawirowało. Bardzo się zdziwiłem, gdyż uważałem ten kurs jako wręcz idealny. Całkowicie zmieniłem przez to swoje podejście i myślenie. Mam teraz mieszane uczucia, bo chciałem zacząć C++ i zastanawiam się czy przerobić ten kurs u Pana Zelenta czy zacząć od czegoś innego. Jeśli ktoś z was może mi podpowiedzieć w którą stronę uderzyć to będę wdzięczny. Dziękuje za tego linka, bo znacznie zmienił mój mindset.

komentarz 30 września 2016 przez ScriptyChris Mędrzec (190,190 p.)

To zależy czy chcesz programować rzeczy po stronie frontu, backendu - czyli ogólnie web - czy wolisz kodzić aplikacje systemowe i/lub gry. Na tym forum już wielokrotnie było powiedziane, żeby uczyć się języka/technologii w której wiesz, że chcesz robić i Ci się podoba. Jeśli chcesz tłuc strony i aplikacje dla przeglądarki to nauka C++ Tobie się nie przyda - ucz się od razu HTML/CSS/JS.

Wg mnie kurs Mirosława Zelenta z C++ jest dobry na sam start - na rozruch, aby w bardzo przystępnej formie ogarnąć o co chodzi i przyswoić absolutne podstawy programowania (zmienne, pętle, ify, funkcje, obiektowość) - ale dobrych praktyk się tam nie nauczysz. Jeśli o mnie chodzi, to pełną wypowiedź na temat kursów MZ możesz przeczytać tutaj: http://forum.pasja-informatyki.pl/179265/ogloszenia-zmiany-w-administracji-nowe-funkcje?show=179783#a179783

Zaś opinie bardziej doświadczonych Pasjonatów i zarazem programistów zawodowych znajdziesz na forum (poszukaj wypowiedzi zwłaszcza userów: Comandeer, efik i event15).

komentarz 30 września 2016 przez Codeboy Stary wyjadacz (12,120 p.)

Powyższych userów już kojarzę ;). Tzn tak, u mnie problem polega na tym ze jeszcze konkretnie nie wiem w którą stronę uderzę, front, backend czy systemowe. Ale C++ chciałem ogarnąć, co najmniej podstawy, bo chciałem zdawać maturę z informatyki. A tam nie ma programowania web :<.

komentarz 5 października 2016 przez Codeboy Stary wyjadacz (12,120 p.)

Wprowadziłem drobne zmiany. Nowy branch dostępny na GitHubie.

Jak masz ochotę i czas to zobacz zmiany i daj znać co i jak ;)

(Pozmieniałem też nazwy zmiennych na angielskie, więc jest "dużo" zmian przez co są mniej czytelne te konkretne ;<)

całość skryptu zamknij w IIFE (chyba, że przepiszesz na ES6, to w bloku)

U mnie IIFE niszczy część funkcji w jakiś sposób(np. dostaje "ReferenceError: postaw is not defined" podczas klikania w pole). Także musiałbym pewnie coś przebudować żeby zamknąć to w IIFE. Na razie dodałem to jako komentarz. Jak wiesz o co chodzi to daj znać ;)

skrypt z <head> przenieś na koniec <body> i wtedy możesz zrezygnować z window.onload

✔ Zrezygnowałem z window.onload ale musiałem i tak wywołać funkcje ręcznie. Mogłem zamknąć tą funkcję w IIFE żeby sama się wykonała ale czy to lepsze rozwiązanie? Tak jest bardziej przejrzyście według mnie.

w pętlach stosujesz zmienne globalne...

✔

gdy tylko możesz nie styluj HTMLa w formie inline lecz nadawaj klasy i styluj w CSS
Nie dawaj w HTMLu atrybutów odpowiedzialnych za obsługę eventów

✔ Tyle że nie wiem jak zaiplementować eventlistenera do pętli bo w ten sposób nie działa. (Na razie dodałem jako komentarz);

Jeśli dodajesz klasę, to możesz to zrobić za pomocąelement.classList.add('nazwa_klasy') i usuwanie poprzez element.classList.remove('nazwa_klasy')

Hmm, ja nie dodaję klasy tylko ją zmieniam

.setAttribute("class", "off");

Więc zamiast tego musiałbym użyć obu:

.classList.remove('pole:hover');
.classList.add('off');

Zrobiłem to lecz czy jest to lepsze rozwiązanie od pojedynczego setAttribute?

stosuj potrójny operator porównania ===

✔

do tego polecam włączyć strict mode

Włączyłem ale nic się nie stało ^^ Jeszcze w wolnej chwili poczytam o tym więcej.

komentarz 5 października 2016 przez ScriptyChris Mędrzec (190,190 p.)
edycja 5 października 2016 przez ScriptyChris

U mnie IIFE niszczy część funkcji w jakiś sposób(np. dostaje "ReferenceError: postaw is not defined" podczas klikania w pole). Także musiałbym pewnie coś przebudować żeby zamknąć to w IIFE.

Której linijki dotyczy ten błąd? Nie widzę ani w HTML ani w JS stringa "postaw".

Funkcja natychmiastowego wywołania zamyka Ci kod w niej zawarty w taki sposób, że nie możesz się do niego dostać z zewnątrz funkcji, również z obiektu window. Przeczytaj jeszcze raz pierwszy podlinkowany artykuł o IIFE.

Zrezygnowałem z window.onload ale musiałem i tak wywołać funkcje ręcznie.

Zapewne masz na myśli funkcję makefields() - przy okazji, nazwy funkcji i zmiennych pisz w formie camelCase. Widzę, że z funkcji tej korzystasz tylko raz - więc nie musisz kodu wewnątrz jej tworzyć jako funkcję. Może to być kod umieszczony na końcu skryptu. Funkcje tworzy się po to, aby kod wewnątrz nich był reużywalny - kod wewnątrz funkcji makefields() nie jest nigdzie używany (powtórzę, korzystasz z niego tylko na początku), zatem nie ma sensu umieszczać go w funkcji. Przerzuć kod z tej funkcji na koniec skryptu.

w pętlach stosujesz zmienne globalne...

✔

Ok, ale nie wiem dlaczego pętle zaczynasz od 1, zamiast od zera.

for (var i=1; i<=9; i++)

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.

Tyle że nie wiem jak zaiplementować eventlistenera do pętli bo w ten sposób nie działa. (Na razie dodałem jako komentarz);

Tutaj?

//nie działo prawidłowo->		document.getElementById(i).addEventListener("click", function(){insert(i);}, false);

Zapewne spotkałeś się właśnie ze zjawiskiem closure. W tym miejscu jeszcze raz odsyłam do: http://benalman.com/news/2010/11/immediately-invoked-function-expression/ i akapitu Saving state with closures

Skoro już zacząłeś bawić się EventListenerami, to nie dodawaj ich w pętli. Wykorzystaj zjawisko event bubbling i zastosuj Event Delegation. W skrócie - podepnij EventListener, do rodzica jakiejś grupy elementów i na nim za pomocą obiektu event oraz pola event.target przechwytuj elementy, które wywołały zdarzenia i na podstawie tego wykonuj na nich operacje.

Hmm, ja nie dodaję klasy tylko ją zmieniam

Zrobiłem to lecz czy jest to lepsze rozwiązanie od pojedynczego setAttribute?

Jakoś specjalnie się nad tym nie zastanawiałem. Zauważ jednak, że element.classList oferuje Ci więcej możliwości manipulowania i wyszukiwania klas na elementach => http://html5doctor.com/the-classlist-api/ korzystanie z jego metod jest też szybsze niż manipulacja klasami przez setAttribute() => https://measurethat.net/Benchmarks/Show/54/0/classname-vs-setattribute-vs-classlist (ale to na większej liczbie elementów)

Żeby "jednym ciurkiem" podmienić klasy (usunąć obecną, dodać inną) przydało by się móc użyć łańcucha metod... Tutaj akurat jQuery daje taką możliwość => http://www.jquery-tutorial.net/introduction/method-chaining/ ale możesz sobie taki chaining dla element.classList dosyć łatwo zaimplementować w czystym JS => http://stackoverflow.com/a/29143197/4983840

Włączyłem ale nic się nie stało ^^ Jeszcze w wolnej chwili poczytam o tym więcej.

W takim razie ciesz się - Twój kod jest "zjadliwy" dla przeglądarki, skoro w konsoli nie wyrzuciło błędów ;) No nie licząc tego o braku referencji do elementu HTML.

Od siebie dodam, że można przyczepić się do tej kobyłki:

function ifwin(who){
	if(document.getElementById("1").innerHTML === document.getElementById("2").innerHTML && document.getElementById("2").innerHTML === document.getElementById("3").innerHTML ||
		document.getElementById("4").innerHTML === document.getElementById("5").innerHTML && document.getElementById("5").innerHTML === document.getElementById("6").innerHTML ||
		document.getElementById("7").innerHTML === document.getElementById("8").innerHTML && document.getElementById("8").innerHTML === document.getElementById("9").innerHTML ||
		document.getElementById("1").innerHTML === document.getElementById("4").innerHTML && document.getElementById("4").innerHTML === document.getElementById("7").innerHTML ||
		document.getElementById("2").innerHTML === document.getElementById("5").innerHTML && document.getElementById("5").innerHTML === document.getElementById("8").innerHTML ||
		document.getElementById("3").innerHTML === document.getElementById("6").innerHTML && document.getElementById("6").innerHTML === document.getElementById("9").innerHTML ||
		document.getElementById("1").innerHTML === document.getElementById("5").innerHTML && document.getElementById("5").innerHTML === document.getElementById("9").innerHTML ||
		document.getElementById("3").innerHTML === document.getElementById("5").innerHTML && document.getElementById("5").innerHTML === document.getElementById("7").innerHTML) 
		win(who);
}

Po pierwsze, dużo za dużo warunków w tym if. Po drugie po co za każdym razem odnosić się do DOM celem znalezienia elementu i odczytania jego wewnętrznego kodu HTML? Zrób sobie wcześniej tablicę składającą się z pól .innerHTML tych wszystkich elementów i po prostu pętlą po nich przejedź (np. czymś pokroju Array.some() => https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/some) i wynik(i) sprawdź w if. W tym miejscu powtórzę - do takich akcji właśnie tworzy się funkcje, aby kod był reużywalny.

Nie przeglądałem Twojego nowego kodu z lupą, ale znalazłem takiego kwiatka:

document.getElementById(nr).onclick="";

Do czego to ma służyć? Jeśli podpinasz EventListener, to możesz też go prawilnie usunąć => https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/removeEventListener

komentarz 5 października 2016 przez Codeboy Stary wyjadacz (12,120 p.)
edycja 5 października 2016 przez Codeboy

Której linijki dotyczy ten błąd? Nie widzę ani w HTML ani w JS stringa "postaw".

Dotyczyło to funkcji postaw(), obecnie nazywa się insert(). (wiedziałem ze będą problemy jak pozmieniam nazwy ;D, ale nie chciałem takiego łamanego języka w kodzie ^^)

Zapewne masz na myśli funkcję makefields() - przy okazji, nazwy funkcji i zmiennych pisz w formie camelCase. Widzę, że z funkcji tej korzystasz tylko raz - więc nie musisz kodu wewnątrz jej tworzyć jako funkcję. Może to być kod umieszczony na końcu skryptu. Funkcje tworzy się po to, aby kod wewnątrz nich był reużywalny - kod wewnątrz funkcji makefields() nie jest nigdzie używany (powtórzę, korzystasz z niego tylko na początku), zatem nie ma sensu umieszczać go w funkcji. Przerzuć kod z tej funkcji na koniec skryptu.

+ so true ;)

Ok, ale nie wiem dlaczego pętle zaczynasz od 1, zamiast od zera.

Wiem, ze w programowaniu powinno się od zera, jednak jak sa divy ponumerowane od 1-9 to mi przyjemniej^^ Wkrótce się przestawię ;)

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ć ...

Zapewne spotkałeś się właśnie ze zjawiskiem closure...

Zauważ jednak, żeelement.classList oferuje Ci więcej możliwości manipulowania i wyszukiwania klas na elementach...

Pozmieniam i doczytam ;)

W takim razie ciesz się - Twój kod jest "zjadliwy" dla przeglądarki, skoro w konsoli nie wyrzuciło błędów ;) No nie licząc tego o braku referencji do elementu HTML.

Yaaaaaay! ;) Co do błędu to występuję tylko gdy zastosuje IIFE. Usuń // sprzed IIFE i sam zobacz. Nie mam pojęcia jak IIFE może na to wpływać.

Od siebie dodam, że można przyczepić się do tej kobyłki:

Wyrzuciłem kobyłkę jakieś 5 minut przed twoim komentarzem ;D. Zobacz jak to zrobiłem i oceń czy jest bardziej przystępnie ;).

document.getElementById(nr).onclick="";

Do czego to ma służyć? Jeśli podpinasz EventListener, to możesz też go prawilnie usunąć

Tego nie zmieniałem z powodu niedziałającego EventListenera, o którym wspominałem. Jeśli tamtego prawidłowo zaimplementuje to i to poprawie ;)

Chyba ze to nie ma związku, ale próbowałem tak:

document.getElementById(i).removeEventListener("click", function(){insert(i);}, false);

(zmieniłem zmienna nr na po prostu "i" bo tamto nie było w ogóle potrzebne)

I nie działa prawidłowo, nadal po wygranej mogę klikać pola.

Dodatkowo zauważyłem ze nie działa hover w:

.classList.remove('field:hover');

Dałem samo field i jest git, ale czy jest możliwość usunięcia samego hovera?

PS: Jestem Ci niezmiernie wdzięczny że poświęcasz dla mnie czas, niewiele to zmienia bo to tylko słowa, ale może twoja dusza będzie zdrowsza ;D

komentarz 6 października 2016 przez niezalogowany

Wiem, ze w programowaniu powinno się od zera, jednak jak sa divy ponumerowane od 1-9 to mi przyjemniej

Ponumeruj divy od 0 : )

var divs = new Array; // niezbyt dobrze
var divs = []; // dobrze

1. Konstruktor można nadpisać.

var Array = function(){ this.very='broken' };

2. Konstruktor poprzez 'przeciążenie' zwraca 'różne wyniki' - nie jest to bynajmniej pożądana cecha w tych przypadku

new Array(2,3) // [2, 3] - OK
new Array(2) // zwróci [2]? - NOPE

3. Literał jest czytelniejszy
4. Literał jest szybszy ( ale tutaj już musiałbyś tworzyć setki tysięcy tablic, żeby poczuć różnicę )

document.getElementById(i).removeEventListener("click", function(){insert(i);}, false);

To nie działa z jednej prostej przyczyny:

new Function == new Function // false

Znaczy to mniej więcej tyle, że funkcja którą chcesz usunąć - nie jest tą samą funkcją którą dodałeś.

Zapakuj

function(){insert(i);}

w zmienną -> i dopiero tę zmienną wysyłaj jako argument do add/remove EventListener.

.addEventListener("click", function(){location.reload();}, false);

Jeśli nie przekazujesz argumentów, po prostu ->

.addEventListener("click", location.reload);

useCapture -> domyślnie jest ustawiony na false

//nie działo prawidłowo-> document.getElementById(i).addEventListener("click", function(){insert(i);}, false);

Asynchroniczność

for (var i=1; i<=9; i++)
{
    fields = fields +'< ... </p></div>';
    document.getElementById("fields").innerHTML=fields;
    document.getElementById(i).addEventListener("click", function(){
        insert(i);
    }, false);
}

W momencie kliknięcia na element -> pętla już dawno się wykonała, a wartość zmiennej i jest równa 10. Stąd, klikając - zawsze wywołasz funkcję insert(10);

var fields ="";

for (var i=1; i<=9; i++)
{
	fields = fields +'<div id="'+i+'" class="field" onclick="insert('+i+')"><p>'+i+'</p></div>';
	document.getElementById("fields").innerHTML=fields;

}

^ czytelność kodu

for (var i=0; i<9; i++)
{

    var field = document.createElement('div');
    field.classList.add('field');
    field.id = i + 1;
    field.textContent = i + 1;
  
	document.getElementById('fields').appendChild(field);
}

var insert = function( e ) 
{
  if( e.target.classList.contains('field') === false ) {
    return; // jeśli nie kliknięto w .field - wyjdź z funkcji
  }

  var nr = parseInt( e.target.id );
  // lub sztuczka z podwójną negacją bitów
  // nr = ~~ e.target.id;
  console.log( nr ) // id klikniętego pola (Number)
}

document.getElementById('fields').addEventListener('click', insert );

Można by jeszcze pisać i pisać o tym kodzie... ale to z jednej przyczny:

brakuje warstwy abstrakcji odpowiedzialnej za komunikację miedzy 'silnikiem' gry, a DOMem. Stan całej rozgrywki nie powinien być oparty o HTML; HTML to tylko 'wyświetlacz' w tym przypadku.

Zacząłem uczyć się javascriptu

Mając to na uwadze - jest super! Zdolny jesteś, to na 'głęboką' wodę z nami : )

komentarz 6 października 2016 przez Codeboy Stary wyjadacz (12,120 p.)
edycja 6 października 2016 przez Codeboy

Witaj, Argeento ;) Miło, że dołączyłeś się ;) PS: Gratuluje zdobycia tytułu eksperta JS ;D

Ponumeruj divy od 0 : )

✔

3. Literał jest czytelniejszy

4. Literał jest szybszy ( ale tutaj już musiałbyś tworzyć setki tysięcy tablic, żeby poczuć różnicę )

✔

To nie działa z jednej prostej przyczyny:

1

new Function == new Function // false

Znaczy to mniej więcej tyle, że funkcja którą chcesz usunąć - nie jest tą samą funkcją którą dodałeś.

Wstawiłem to w ten sposób i nie wykonuje się prawidłowo, o to Ci chodziło? W sumie to chyba nic nie zmieniłem, to jest nadal to co było hm?

var rmInsert= "function(){insert("+i+");}";
document.getElementById(i).removeEventListener("click", rmInsert);

Jeśli nie przekazujesz argumentów, po prostu ->

1

.addEventListener("click", location.reload);

useCapture -> domyślnie jest ustawiony na false

✔ Tyle, że nie mogę dać w ten sposób:

	
.addEventListener("click", location.reload);

Bo wywala "Illegal invocation" , więc zostawiam tak:

.addEventListener("click", function(){location.reload();});

Asynchroniczność
for (var i=1; i<=9; i++)
{
    fields = fields +'< ... </p></div>';
    document.getElementById("fields").innerHTML=fields;
    document.getElementById(i).addEventListener("click", function(){
        insert(i);
    }, false);
}
W momencie kliknięcia na element -> pętla już dawno się wykonała, a wartość zmiennej i jest równa 10. Stąd, klikając - zawsze wywołasz funkcję insert(10);

Hmm, rozumiem, po prostu działa to inaczej niż założyłem a mianowicie że z każdym wykonaniem pętli jest dodawany EventListener do diva. W takim razie jak mógłbym to wykonać?

^ czytelność kodu

for (var i=0; i<9; i++)
{
 
    var field = document.createElement('div');
    field.classList.add('field');
    field.id = i + 1;
    field.textContent = i + 1;
   
    document.getElementById('fields').appendChild(field);
}
 
var insert = function( e ) 
{
  if( e.target.classList.contains('field') === false ) {
    return; // jeśli nie kliknięto w .field - wyjdź z funkcji
  }
 
  var nr = parseInt( e.target.id );
  // lub sztuczka z podwójną negacją bitów
  // nr = ~~ e.target.id;
  console.log( nr ) // id klikniętego pola (Number)
}
 
document.getElementById('fields').addEventListener('click', insert );

Z tym się wstrzymam, bo dużo nowych funkcji i musiałbym sporo przebudowywać tego co mam. Wiem, że to niweluje moje poprzednie pytanie, ale dziś już nie dam rady :(.

Można by jeszcze pisać i pisać o tym kodzie... ale to z jednej przyczny:

brakuje warstwy abstrakcji odpowiedzialnej za komunikację miedzy 'silnikiem' gry, a DOMem. Stan całej rozgrywki nie powinien być oparty o HTML; HTML to tylko 'wyświetlacz' w tym przypadku.

Jeśli masz tylko siły, chęci i czas to nie przestawaj! Jestem chętny do współpracy :).

Precyzując, chodzi Ci o to, że stan divów jest sprawdzany na poziomie HTML?
A nie dajmy na to na jakieś tablicy gdzie był by wprowadzany status pola i wtedy bym sprawdzał warunki na tablicy?

Mając to na uwadze - jest super! Zdolny jesteś, to na 'głęboką' wodę z nami : )

Podbudowałeś mnie tymi słowami i wywołałeś delikatny uśmiech ;) Ale nie jestem przekonany, że to prawda ;D Ale bardzo chętnie ;)

Dzięki za rady i poświęcony czas! Chetnie przyjmę więcej ;D

komentarz 6 października 2016 przez ScriptyChris Mędrzec (190,190 p.)

var rmInsert= "function(){insert("+i+");}";
document.getElementById(i).removeEventListener("click", rmInsert);

Nie, nie, nie. To niebezpiecznie zbliża się do skorzystania z eval. Zapisz po prostu:

document.getElementById(i).removeEventListener("click", rmInsert);

function rmInsert() {
   /** kod */
}

Usuwanie EventListenera jest możliwe, gdy podasz jako parametr tą samą funkcję (tzn. referencję do niej) co funkcja, którą przekazałeś przy dodawaniu EventListenera (to przecież jest napisane w podlinkowanym docsie).

Calling removeEventListener() with arguments that do not identify any currently registeredEventListener on the EventTarget has no effect.

Jeśli przekazujesz anonimową funkcję, czyli

element.addEventListener( 'click', function() { /** wnętrze funkcji anonimowej - nie masz do tego referencji */ } );

, to później nie masz do niej dostępu - nie "powiesz" JSowi, której funkcji użyłeś, bo nie masz referencji po której do tej funkcji możesz się odnieść. Jeśli nazwiesz funkcję, albo zapiszesz ją w formie function expression ( http://lucybain.com/blog/2014/js-anonymous-referenced-declared-functions/ ) czyli:

var awesomeFunction = function() { /** awesome code */ };

element.addEveneListener( 'click', awesomeFunction );

, to masz wtedy referencje do funkcji anonimowej, o nazwie awesomeFunction - jeśli nie nazwiesz funkcji, to potem się do niej nie odniesiesz. Anonimowe funkcję są przydatne, gdy chcesz w locie stworzyć funkcję i posłać jako callback. Jednak zbyt wiele anonimowych funkcji może być trudne w "utrzymaniu". Anonimowe funkcje są trudniejsze w debugowaniu, bo w konsoli nie wypisze Ci nazwy funkcji, która się wykrzaczyła - dostaniesz jeśli już linijkę kodu, w której coś się zepsuło. W przypadku funkcji nazwanych (lub z referencją) dostaniesz jej nazwę i szybciej ją zlokalizujesz.

Bo wywala "Illegal invocation" , więc zostawiam tak:

Jeśli chcesz odpiąc potem ten listener, to po prostu przekaż tam nazwę funkcji:

element.addEventListener( "click", refreshPage );

function refreshPage() {
    location.reload();
}

komentarz 7 października 2016 przez Codeboy Stary wyjadacz (12,120 p.)

Tego nie zmieniałem z powodu niedziałającego EventListenera, o którym wspominałem. Jeśli tamtego prawidłowo zaimplementuje to i to poprawie ;)

Czyli nie myliłem się tak bardzo ^^. Niedługo ogarnę tą sprawę ;) A odpowiesz jeszcze do poprzedniej mojej odpowiedzi do twoich wskazówek? O to nie działające IIFE najbardziej mi chodzi ;) Aktualnie gdy zamknę wszystko w IIFE dostaję Uncaught ReferenceError: insert is not defined.

komentarz 7 października 2016 przez niezalogowany

Dzięki Gustaw24 : ) Ekspert to tylko na poziomie tego forum : p Ekspertem w JS bym się z pewnością nie nazwał.

.addEventListener("click", location.reload);
wywala "Illegal invocation"

Welp.. reload potrzebuje obiektu this = window.location, wywołując metodę z tej pozycji -> podstawiamy mu pod this - kliknięty element. I się psuje : /

Cóż.. to w sumie funkcja anonimowa nie jest złym pomysłem.

Chociaż zawsze można reload 'zmusić' do działania : )

.addEventListener('click' , location.reload.bind( location ) )

Hmm, rozumiem, po prostu działa to inaczej niż założyłem a mianowicie że z każdym wykonaniem pętli jest dodawany EventListener do diva. W takim razie jak mógłbym to wykonać?

Zamiast wysyłać parametr, pobierać id z klikniętego diva,

function insert()
{
    // Zamiast insert(i)
    // Mniej więcej coś takiego:
    var i = parseInt( this.id );
 
	//Porownanie zawartosci poczatkowej diva
	
	var getState = document.getElementById(i).innerHTML;
	var state="<p>"+i+"</p>";
	
	if (round%2==0 && getState == state) {

ogólnie pod this, masz kliknięty element (możesz pisać this zamiast document.getElementById(i) ) - tylko jeden warunek, nie możesz przypisać eventu clickniecia w HTMLu ( wtedy this będzie wskazywało na obiekt window )

(Krzycho nie krzycz jeszcze na mnie : p event delegation w swoim czasie )

Precyzując, chodzi Ci o to, że stan divów jest sprawdzany na poziomie HTML?
A nie dajmy na to na jakieś tablicy gdzie był by wprowadzany status pola i wtedy bym sprawdzał warunki na tablicy?

Bardziej myślałem, o przepisaniu tego na wersję obiektową

komentarz 7 października 2016 przez Codeboy Stary wyjadacz (12,120 p.)

ogólnie pod this, masz kliknięty element (możesz pisać this zamiastdocument.getElementById(i) ) - tylko jeden warunek, nie możesz przypisać eventu clickniecia w HTMLu ( wtedy this będzie wskazywało na obiekt window )

No a właśnie chodzi nam o podpięcie EventListenera więc nie da rady z tym this. Ale dobrze wiedzieć jak sie zachowuje this w tym wypadku ;)

(Krzycho nie krzycz jeszcze na mnie : p event delegation w swoim czasie )

Przeczytałem o event delegation i udało mi się to zaimplementować w ten sposób:

document.getElementById("fields").addEventListener("click",function(e) 
	{
	if (e.target && e.target.matches("div.field")) 
	{
		var i = e.target.id;
		insert(i);
	}
	});

Rozumiem jak to działa ale nie ogarniam jeszcze poszczególnych kroków, muszę to bardziej przestudiować ;)

field.textContent = i;

Czy to jest jakos inaczej kodowane czy cos? Bo jak to alertuje to sa krzaczki.

komentarz 7 października 2016 przez niezalogowany

Jak coś wstawiaj zawsze aktualną wersję na gita

ogólnie pod this, masz kliknięty element (możesz pisać this zamiastdocument.getElementById(i) ) - tylko jeden warunek, nie możesz przypisać eventu clickniecia w HTMLu ( wtedy this będzie wskazywało na obiekt window )

No a właśnie chodzi nam o podpięcie EventListenera więc nie da rady z tym this. Ale dobrze wiedzieć jak sie zachowuje this w tym wypadku ;)

this nie zadziała tylko kiedy użyjesz w HTMLu atrybutu onclick -> własnie o to chodzi, żeby przepisać to na EventListener i użyć this : ) -> jeśli masz już zakodowane Event Delegation, tym lepiej. Można olać thisy.

field.textContent = i;

?? https://developer.mozilla.org/en-US/docs/Web/API/Node/textContent

Po prostu w diva field wpisujesz numer

komentarz 7 października 2016 przez ScriptyChris Mędrzec (190,190 p.)

O to nie działające IIFE najbardziej mi chodzi ;) Aktualnie gdy zamknę wszystko w IIFE dostaję Uncaught ReferenceError: insert is not defined.

Błędu przy zastosowaniu dopatrywałbym się w tym miejscu:

fields = fields +'<div id="'+i+'" class="field" onclick="insert('+i+')"><p>'+i+'</p></div>';

Nie wiem o co dokładnie chodzi, ale ten onclick w pętli brzydko wygląda.

Chętnie bym się dowiedział dlaczego tutaj onclick głupieje (tzn. tak jakby nie miał dostępu do funkcji insert, może kolejność stworzenia elementu vs podpięcie funkcji do onclick jest tu jakaś inna) - może @Comandeer tu zajrzy i coś podpowie.

Jedyne co udało mi się znaleźć na ten temat: http://stackoverflow.com/a/17378538/4983840

a konkretnie:

The reason is that userscripts operate in a sandbox ("isolated world"), and onclick operates in the target-page scope and cannot see any functions your script creates.

Czyli onclick nie widzi funkcji, które się stworzyło - bo scope'y są inne. Tak mi się wydaje, może ktoś to lepiej wyjaśni. :)

komentarz 7 października 2016 przez niezalogowany

O to nie działające IIFE najbardziej mi chodzi ;) Aktualnie gdy zamknę wszystko w IIFE dostaję Uncaught ReferenceError: insert is not defined.

Jeśli zamkniesz funkcję insert w IIFE ( stworzysz nowego scopa, do którego nie ma dostępu z zewnątrz ) - JS w HTMLu <onclick=insert();> 'nie widzi' tej funkcji -> stąd błąd.

W sumie o to chodzi w IIFE : )

komentarz 7 października 2016 przez ScriptyChris Mędrzec (190,190 p.)

@niezalogowany Ah, faktycznie. Gdy doda się EventListener, to JS widzi dany element HTML (przecież pobieramy go z poziomu JS), a tutaj całość jest zamknięta w IIFE (czyli nie widoczna przez window ani też z DOM), więc podpinając tą funkcję do HTMLowego onclick => HTML nie wie skąd ona się wzięła (nie ma dostępu, bo przypięta funkcja jest w IIFE).

Myślałem, że tu chodzi o jakieś poziomy w DOM albo o scope'y skryptów (to co zacytowałem).

komentarz 8 października 2016 przez Codeboy Stary wyjadacz (12,120 p.)
edycja 8 października 2016 przez Codeboy

Jak coś wstawiaj zawsze aktualną wersję na gita

Staram się ;) Ale wtedy akurat byłem w szkole i sobie to ogarniałem ^^ Już wrzucę najnowsze nie długo :)

this nie zadziała tylko kiedy użyjesz w HTMLu atrybutu onclick -> własnie o to chodzi, żeby przepisać to na EventListener i użyć this : ) -> jeśli masz już zakodowane Event Delegation, tym lepiej. Można olać thisy.

Miałem problem nawet z EventListenerem , ale niestety nie drążyłem tematu i zrobiłem Event Delegation ^^ ?

1

field.textContent = i;

?? https://developer.mozilla.org/en-US/docs/Web/API/Node/textContent

Po prostu w diva field wpisujesz numer

Precyzując, gdy w diva włożyłem taką wartość:

field.textContent ="<p>"+i+"</p>";

I nie działało mi sprawdzanie zawartości diva, wiec dla testu wyciagnalem innerHTML z któregoś divów a tam zamiast <p>"+i+"</p> jest <p>0</p> ^^ Znacznik <p> dobrze by było zostawić, bo dzięki temu wyłączam widzialność numerów id w polach.

EDIT: Znalazłem prostsze rozwiązanie, nadałem to na pola i z głowy:

color: transparent;

Mimo wszystko ciekawy jestem skąd te krzaczki ;D

Event Listenery juz podpięte, IIFE działa, większość waszych rad chyba zaimplementowana ;) Jutro pracuję dalej ;)

komentarz 9 października 2016 przez Codeboy Stary wyjadacz (12,120 p.)

Przeczytałem o event delegation i udało mi się to zaimplementować w ten sposób:
document.getElementById("fields").addEventListener("click",function(e) 
	{
	if (e.target && e.target.matches("div.field")) 
	{
		var i = e.target.id;
		insert(i);
	}
	});

Tego nie wspiera kochana przeglądarka od microsoftu :''''''') ~Edge.

Object doesn't support property or method 'matches'

komentarz 9 października 2016 przez ScriptyChris Mędrzec (190,190 p.)

Spróbuj ciachnąć polyfilla: https://developer.mozilla.org/en/docs/Web/API/Element/matches#Polyfill

komentarz 9 października 2016 przez niezalogowany

Krzycho... ale po co? matches to metoda stringa - nie obiektu (inna sprawa, że jeśli nie potrzebujesz regexpa, lepiej skorzystać z indexOf )

if( e.target.classList.contains('field') === false ) {
    return; // jeśli nie kliknięto w .field - wyjdź z funkcji
}

komentarz 10 października 2016 przez Codeboy Stary wyjadacz (12,120 p.)
edycja 10 października 2016 przez Codeboy

Nie rozumiem o czym piszesz i jak to zaimplementować, nie widzę też związku tego kodu co dodałeś a moim problemem, naprostujesz? :)

Polyfill działa.

komentarz 16 października 2016 przez Codeboy Stary wyjadacz (12,120 p.)
edycja 16 października 2016 przez Codeboy

Argeento, zwątpiłeś? Nie zostawiaj mnie ;D

Jednak domyśliłem się :)

komentarz 16 października 2016 przez Codeboy Stary wyjadacz (12,120 p.)

Wrzuciłem nowe repozytorium, macie jeszcze jakieś uwagi? Z tym powoli będę kończył ;)
Zrobiłem z kolegą coś nowego, Game of life, pewnie kojarzycie, wrzucę niedługo na forum pod code review ;D

komentarz 16 października 2016 przez niezalogowany

Pomyliłem match z matches, w dodatku nie znałem tej metody

komentarz 16 października 2016 przez Codeboy Stary wyjadacz (12,120 p.)

W każdym razie pomogłeś, zrobiłem po swojemu i dużo linijek wypadło i polyfill tez :)

Przy okazji, zobaczcie jak możecie czy wam na telefonach śmiga wszystko, wrzuciłem na stronkę dla wygody ;>
http://www.kolko-i-krzyzyk.awsome.pl

komentarz 16 października 2016 przez niezalogowany

	if( e.target.classList.contains('field') === false ) {
    return;
	}
	else
	{
		var i = e.target.id;
		insert(i);
	}

^ co Ty tam tworzysz ; )

if( e.target.classList.contains('field')  ) {
    insert( e.target.id );
}

for(var i=0; i<=8; i++) 
{
	document.getElementById(i).removeEventListener("click", insert);
	document.getElementById(i).classList.remove('field');
	document.getElementById(i).classList.add('off');
}

Przeszukiwanie DOM za każdym razem - jest 'kosztowne' dla procesora. Znacznie ładniejszym rozwiązaniem byłoby zapakowanie wszystkich pól do tablicy

var field = document.querySelectorAll('.field');

później tylko:

for(var i=0; i<=8; i++) 
{
	field[i].removeEventListener("click", insert);
	field[i].classList.remove('field');
	field[i].classList.add('off');
}

*uważaj, zmienna field może Ci namieszać przy tworzeniu tablicy.. albo na odwrót. Takie życie gdy nie masz scopa z funkcji / oddzielnej klasy

*zawsze parsuj zmienne przychodzące z DOM API // parseInt( e.target.id );

komentarz 16 października 2016 przez niezalogowany

function insert(i)
{
	//Porownanie zawartosci poczatkowej diva
	
	if (round%2==0) {
		
		document.getElementById(i).innerHTML="<img src='img/krzyzyk.png'>";
		click.play();
		
		round++;
		if(round>=3 && round<=10) ifWin("x");
		return;
		
	}
	if(round%2!=0) {
		
		document.getElementById(i).innerHTML="<img src='img/kolko.png'>";
		click.play();
		
		round++;
		if(round>=3 && round<=10) ifWin("o");
		return;
		
	}
	
}

DRY

function insert(i)
{
	click.play();
	round++;

	if ( round % 2 )
	{
		document.getElementById(i).innerHTML="<img src='img/kolko.png'>";
		if(round>=3 && round<=10) ifWin("o");
	}
	else 
	{
		document.getElementById(i).innerHTML="<img src='img/krzyzyk.png'>";
		if(round>=3 && round<=10) ifWin("x");
	}
}

( Btw.. wcięcia popraw w kodzie, bo wygląda to strasznie )

if(round>=3 && round<=10) ifWin("x");

logika sprawdzająca wygraną powinna być w całości przeniesiona do funkcji sprawdzającej kto wygra

komentarz 16 października 2016 przez Codeboy Stary wyjadacz (12,120 p.)
edycja 16 października 2016 przez Codeboy

if( e.target.classList.contains('field') === false ) {

return;

}

else

{

var i = e.target.id;

insert(i);

}

^ co Ty tam tworzysz ; )

Rzeczywiście to było nie potrzebne, ale działało :D Zmyliły mnie wcześniejsze metody, wstawiające kolko/krzyżyk ^^

( Btw.. wcięcia popraw w kodzie, bo wygląda to strasznie )

Hmmm, nie za bardzo wiem o czym mówisz, dla mnie to wygląda całkiem przejrzyście ^^. Musiałbyś bardziej sprecyzować, bo nie wiem jakbym miał to zrobić ;)

Przeszukiwanie DOM za każdym razem - jest 'kosztowne' dla procesora. Znacznie ładniejszym rozwiązaniem byłoby zapakowanie wszystkich pól do tablicy

Co masz na myśli mówiąc "za każdym razem"? To jest wykonywane tylko raz, jeśli zaszła wygrana.

if(round>=3 && round<=10) ifWin("x");

logika sprawdzająca wygraną powinna być w całości przeniesiona do funkcji sprawdzającej kto wygra

Mówiąc "logika" masz na myśli co? to round>=3 && round<=10? Zacytowałeś mi wywołanie samej funkcji sprawdzającej, nie za bardzo rozumiem.

Musisz trochę jaśniej opisywać problemy bo jeszcze nie jestem taki biegły w tych rzeczach ;)

komentarz 16 października 2016 przez niezalogowany

4-67 - jedno wcięcie za mało
19-24 - jedno wcięcie za dużo
76 - jedno wcięcie za dużo
86-93 - jedno wcięcie za mało

if (round%2==0) {

ifWin(who){

insert(i)
{

if(round>9) win("draw");

rozpoczynasz nowe bloki na 4* różne sposoby, wybierz jeden i bądź konsekwentny

komentarz 16 października 2016 przez Codeboy Stary wyjadacz (12,120 p.)

Okey, już rozumiem ;)

PS: edytowałem wcześniejszy komentarz, uzupełniłem pytanie ;)

komentarz 16 października 2016 przez niezalogowany

Co masz na myśli mówiąc "za każdym razem"? To jest wykonywane tylko raz, jeśli zaszła wygrana.

W funkcji insert, szukany jest element, za każdym razem, gdy ktoś się ruszy.

W funkcji ifwin, szukane są elementy x9, prawie za każdym razem, gdy ktoś się ruszy.

komentarz 16 października 2016 przez niezalogowany

Mówiąc "logika" masz na myśli co? to round>=3 && round<=10? Zacytowałeś mi wywołanie samej funkcji sprawdzającej, nie za bardzo rozumiem.

Tak - funkcja sprawdzająca powinna sprawdzać kto wygrał, niezależnie od tego, która jest runda.

komentarz 16 października 2016 przez Codeboy Stary wyjadacz (12,120 p.)

W funkcji insert, szukany jest element, za każdym razem, gdy ktoś się ruszy.

W funkcji ifwin, szukane są elementy x9, prawie za każdym razem, gdy ktoś się ruszy.

Tak, ale zacytowałeś czyszczenie "aktywności" pól, a to wykonywane jest wtedy gdy zajdzie wygrana ;) A to chyba co innego niż to co teraz opisujesz.

Tak - funkcja sprawdzająca powinna sprawdzać kto wygrał, niezależnie od tego, która jest runda.

Ten warunek dałem po to bo jaki sens ma sprawdzanie wygranej gdy runda < 3 ?

komentarz 16 października 2016 przez ScriptyChris Mędrzec (190,190 p.)

Przeszukiwanie DOM za każdym razem - jest 'kosztowne' dla procesora. Znacznie ładniejszym rozwiązaniem byłoby zapakowanie wszystkich pól do tablicy

Co masz na myśli mówiąc "za każdym razem"? To jest wykonywane tylko raz, jeśli zaszła wygrana.

Argeento zapewne miał na myśli to, że z każdą iterację pętli odnosisz się do danego elementu trzykrotnie i w każdej iteracji odnosisz się do DOM. Czyli:

for(var i=0; i<=8; i++) 
{
    document.getElementById(i).removeEventListener("click", insert);
    document.getElementById(i).classList.remove('field');
    document.getElementById(i).classList.add('off');
}

Argeento wskazał Ci, abyś najpierw zrobił sobie tablicę zawierającą elementy, po których będziesz sobie w pętli iterował (będziesz je mieć już w pamięci, przez co z każdą iteracją JS nie będzie musiał sięgać do DOM, aby je pobrać).

Poza tym, po co trzykrotnie pobierać element, aby wziąc jego classList albo coś innego, skoro możesz pobrać element, umieścić go w zmiennej i na tej zmiennej działać. Czyli:

var element = document.getElementById(i);
element.removeEventListener("click", insert);
element.classList.remove('field');
element.classList.add('off');

, pobierasz element do zmiennej raz, a potem tylko odnosisz się do niego przez referencje (jako, że JS elementy DOM traktuje jako obiekty, a do tych JS z kolei odnosi się przez referencje) poprzez tą zmienną i robisz z tym elementem co tam chcesz - przez co nie musisz za każdym razem odnosić się do DOM, aby sobie coś z elementu odczytać czy w nim zmienić.

rozpoczynasz nowe bloki na 4* różne sposoby, wybierz jeden i bądź konsekwentny

Owszem, wybór jednego stylu pisania jak najbardziej, ale czytałem coś takiego: https://github.com/airbnb/javascript#blocks

i jest tam napisane, że jedno linijkowe instrukcje po if można pisać zarówno w jednej linijce jak i wielu :) Więc IMHO, jak komu wygodniej. Chociaż rzeczywiście lepiej trzymać się jednego stylu zapisu. Tak tylko nawiasem rzucam.

Przypomniałem też sobie, że muszę refaktoryzować kod na GitHub, bo chyba nie spełnia tych wymagań.

komentarz 16 października 2016 przez niezalogowany

if(round>=3 && round<=10) ifWin("o");

Zatem niech sprawdzanie licznika rund będzie pierwszą czynnością w ifwin. Funkcja insert jest od wstawiania, funkcja ifwin od sprawdzania. Niepotrzebnie to mieszasz.

Druga sprawa, po co sprawdzać czy aby na pewno nie jest runda 11 lub dalej

Więc IMHO, jak komu wygodniej

IMO bycie konsekwentnym, to bycie konsekwentnym. Pod ifem jednak zawsze spodziewam się zamknięcia jego bloku.

komentarz 16 października 2016 przez Codeboy Stary wyjadacz (12,120 p.)

Dzięki Krzycho za wytłumaczenie ;)

Przypomniałem też sobie, że muszę refaktoryzować kod na GitHub, bo chyba nie spełnia tych wymagań.

??? ;D

komentarz 16 października 2016 przez Codeboy Stary wyjadacz (12,120 p.)

Przeszukiwanie DOM za każdym razem - jest 'kosztowne' dla procesora. Znacznie ładniejszym rozwiązaniem byłoby zapakowanie wszystkich pól do tablicy

Użyłem pętle globalnie i zastosowałem to do wszystkich możliwych funkcji ;)

*uważaj, zmienna field może Ci namieszać przy tworzeniu tablicy.. albo na odwrót. Takie życie gdy nie masz scopa z funkcji / oddzielnej klasy

Wydaję się, że nie namieszało ;)

*zawsze parsuj zmienne przychodzące z DOM API // parseInt( e.target.id );

Hmmm, ale tylko wtedy kiedy ta zmienna ma być liczbą prawda? ;)
W moim kodzie tylko to co napisałeś do sparsowania, nie mylę się?

Zatem niech sprawdzanie licznika rund będzie pierwszą czynnością w ifwin. Funkcja insert jest od wstawiania, funkcja ifwin od sprawdzania. Niepotrzebnie to mieszasz.

if (round%2==0) {
			
			field[i].innerHTML="<img src='img/krzyzyk.png'>";
			if(round>=3) ifWin("x");
			return;
			
		}

Nie za bardzo rozumiem gdzie tu się miesza, przecież po prostu jest warunek kiedy wywołać sprawdzenie, czyli kiedy runda >3. Chyba, że chodzi Ci o to żeby całkiem wyrzucić, z funkcji wstawiania(insert), wywołanie funkcji sprawdzającej(ifWin) i zrobić to osobno.