Wątek przeniesiony 2016-09-24 12:08 z Webmastering przez dzek69.

Ocena kodu JS i wydajności w Tic Tac Toe AI opartego na algorytmie minimax

0

Cześć!

Chciałbym prosić o ocenę kodu oraz wydajności mojego ostatniego projektu.

Na pomysł na ten projekt wpadłem, gdy męczyłem się z implementacją 'komputera' w kółko i krzyżyk.

Pomyślałem, że dużo łatwiej będzie mi to zrobić jeśli rozdzielę całą abstrakcję tego algorytmu i zamknę w oddzielnym module. Uznałem, że to co mi wyszło może być dla kogoś użyteczne dlatego opakowałem to w ładne API i mini-dokumentację w pliku README.

Tutaj można przetestować jak sprawuje się AI w mojej implementacji TicTacToe: link

A tutaj link do repozytorium tic-tac-toe-ai na GitHubie: link

Pozdrawiam i z góry dzięki za odpowiedź!

1

duplikacja kodu w init:
https://github.com/pietrzakacper/tic-tac-toe-ai/blob/master/source/init.js

jeszcze większa duplikacja w module validation:
https://github.com/pietrzakacper/tic-tac-toe-ai/blob/master/source/validation.js
funkcje aiCharacter(), playerCharacter(), oraz startingCharacter() praktycznie są takie same, różnią się małym szczegółem (można chociażby wydzielić powtarzający się kod do jednej funkcji). W zasadzie można zrobić 1 funkcję character zamiast 3, bo jedyna różnica między tymi funkcjami to sprawdzanie tego warunku:

			if (character === aiCharacter) {
				console.log('AI ERROR: Player character' + character + ' cannot be the same as ai character ' + aiCharacter + '!');
				return false;
			}

a to można po prostu wypieprzyć wyżej, do kodu wywołującego.

			return board.reduce(function(previousValue, currentValue){
				return currentValue === character ? ++previousValue : previousValue;
			}, 0);

Ten kod, choć zdaje się być poprawny, to mylący. Bo ++ sugeruje, że robisz mutację zmiennej previousValue, jednak logika tego wyrażenia reduce jest inna i równie dobrze możesz napisać:

			return board.reduce(function(previousValue, currentValue){
				return currentValue === character ? previousValue + 1 : previousValue;
			}, 0);
0

Dzięki za odpowiedź, już poprawione, muszę jeszcze potestować czy działanie kodu się nie zmieniło:
init.js: przed i po

validation.js: przed i po

1

muszę jeszcze potestować czy działanie kodu się nie zmieniło

Dorób unit testy i sprawdź czy przechodzą zarówno przed jak i po zmianie.

0

Dobry pomysł, dzięki :)

1

Ogólnie widzę kilka zasadniczych problemów z kodem:

  • ES5 i kombinacje z module pattern zamiast po prostu użyć ES6 i nie zaciemniać kodu,
  • Duża złozoność cyklomatyzna, 10 ifów w funkcji?? Toż to horror - świadczy to o złej architekturze rozwiązania.
  • tam gdzie wrzuciłeś komentarze powinieneś wydzielić funkcje, np:
var characters = ['x', 'o'];
//check if one of players won
for (var i = 0; i < 2; ++i) {
	if (hasWon(characters[i], board)) {
		return characters[i] + '-won';
	}
}
//check if there are stil empty fields left
for (var i = 0; i < 9; ++i) {
	if (board[i] === 'e') {
		return 'not-end';
	}
}
  • wygląda na to, że nie uzywasz żadnych narzędzi sprawdzających jakość kodu (widać po błedach, np podwójnej deklaracji var i w wyżej zacytowanym kodzie), zacznij, polecam ESLint i https://github.com/airbnb/javascript
  • console.log() do obsługi błedów - niedopuszczalne,
  • ogólnie kod niepotrzebnie rozwleczony, naiwny, bardzo imperatywny z mnósttwem mutacji - kombinacja ifów i forów (z magic numbers), przez co mało czytelny,
  • brak testów,
  • brak npma.

Nie ma tragedii ale jeszcze dużo przed Tobą.

1

Tak się składa, że używam ESlinta. Na jakiej podstawie sądzisz, że nie używam ?

Bo widzę błędy składniowe, jak używasz ESLinta, to znaczy, że konfiguracja jest uboga - dodaj coś w stylu podlinkowanej przeze mnie konfiguracji.

Jeśli chodzi ci o użycie konsoli, to celowo na nią pozwoliłem. Nie mam pomysłu jak inaczej zakomunikować użytkownikowi , że wprowadził błędne dane.

Rzucaj errory ( https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/Error ), najlepiej customowe ( https://github.com/bjyoungblood/es6-error ).

Pisząc brak npm-a masz na myśli, że nie udostępniam pliku package.json ?

Tak.

0

Dzięki wielkie za poświęcony mi czas :)

Nie ma tragedii ale jeszcze dużo przed Tobą.

Zmotywowało mnie to :)

1 użytkowników online, w tym zalogowanych: 0, gości: 1