Prośba o code review - gra w kości napisana w JavaScript

Cześć!
Chciałbym was prosić o code-review projektu. Pisałem go w czystym JavaScriptcie, z HTML + SCSS. Jest to mój pierwszy taki większy projekt.
Zależy mi bardzo na waszej opinii nie tylko dot. kodu, ale też nazewnictwa commitów (chociaż sam zdaję sobie sprawę, że jest tam kilka kruczków, które nie powinny się znaleźć).

Kod: https://github.com/damian08k/Dice
Live: https://damian08k.github.io/Dice/

Z góry bardzo dziękuję za każdy komentarz!

Nie działa na Safari 13.1.1 na macOS Mojave.

SyntaxError: Unexpected token '='. Expected an opening '(' before a method's parameter list.

Przypuszczam, że dlatego, że korzystasz z eksperymentalnego ficzera JS:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Classes/Public_class_fields

i piszesz coś w stylu:

 startGame = () => {

Zastanów się, czy w ogóle ci potrzebna taka notacja tutaj - moim zdaniem ten akurat ficzer z nowego JSa jest przereklamowany i nadużywany. Ale jeśli już go chcesz używać, to wypadałoby transpilować kod. Tylko mam wrażenie, że nie masz tutaj z takiej notacji żadnej korzyści.
A tu masz, jak poszczególne przeglądarki to wspierają https://kangax.github.io/compat-table/esnext/

Co do samego kodu - to strasznie go dużo jest. Kości naprawdę są aż tak skomplikowaną grą do implementacji?
Zgaduję, że po prostu kod jest przeinżynierowany - np. tworzysz ileś getterów getFirstPlayerLowerSum itp. po to, żeby ich użyć jeden raz w module Statistics. Ogólnie kod sprawia wrażenie pisanego w Javie :|

Jakby jeszcze to przekombinowanie przynajmniej sprawiało, że kod jest elastyczny, ale też nie bardzo - np. zamiast mieć jakąś tablicę graczy, to zakładasz, że graczy będzie maks dwóch i robisz zmienne typu firstPlayerName, secondPlayerName. Czyli nie zrobisz z tego łatwo gry na 3 graczy.

No i mieszasz w tych samych klasach logikę gry (ogólne zasady) z GUI, kodem, który odwołuje się do elementów DOM. To słaby sposób pisania - lepiej byłoby to oddzielić i osobno mieć model(logikę, dane) i osobno mieć widok/GUI. Wtedy wszystko ma swoje miejsce, bardziej jest czytelnie, łatwiej można edytować i można łatwo podmienić logikę albo GUI.

Tym niemniej żeby było pozytywnie, to wygląd gry jest wg mnie spoko - grafika jest minimalistyczna, ale estetyczna, ładny font i przyciski.

@LukeJL: Hej, dzięki za komentarz!

1. Ten błąd w zasadzie nie wiem czym jest spowodowany. Właśnie nie korzystałem z czegoś takiego, raczej unikałem eksperymentalnych rzeczy i nie stosowałem nawet tych co są w stage 3/4 nowego ESa. Sprawdzę to.
2. Tak, to jest lekka pięta achillesowa tego projektu - muszę przepisać metody ze składnią arrow function. Dzięki, że mi to wytknąłeś. Po prostu zacząłem tego używać jak to poznałem i jak jeszcze tworzyłem coś w paradygmacie funkcyjnym to (np. jest tak w pliku rules.js) to korzystam tylko z tej notacji.
3. Jeśli chodzi o skomplikowanie implementacji - nie mam pojęcia, to mój pierwszy większy projekt, starałem się go napisać najprościej i najlepiej jak umiałem, ale i tak sprawił mi zarówno kilka problemów, a także świetnie się przy nim bawiłem.
4. Jeśli chodzi o wrażenie pisania w Javie - to może być wina książki "Czysty kod", którą czytałem. Wziąłem sobie do serca to o czym autor tam pisał (a on tam operował na Javie właśnie). Rozumiem, że takie rozwiązania w JSie raczej nie są niezbędne?
5. Jeśli chodzi o zmienne graczy - w domyśle miało to być tylko na max dwie osoby, takie było "założenie projektu". Nigdy nie zakładałem rozbudowy na więcej w przyszłości, stąd też taka decyzja.
6. Mógłbyś powiedzieć mi w jaki sposób oddzielić logikę z GUI? Nie jestem pewny dokładnie o co Ci chodzi - czy o CSSowe zmiany (np. stylów, kolorów), czy o pobieranie elementów z DOM? Na to powinna być jakaś jedna oddzielna klasa/moduł?

2. Tak, to jest lekka pięta achillesowa tego projektu - muszę przepisać metody ze składnią arrow function. Dzięki, że mi to wytknąłeś. Po prostu zacząłem tego używać jak to poznałem i jak jeszcze tworzyłem coś w paradygmacie funkcyjnym to (np. jest tak w pliku rules.js) to korzystam tylko z tej notacji.

Tylko to są dwie różne notacje. JavaScript jest trochę taką łataniną i wiele rzeczy wygląda podobnie, a ma inne znaczenie dla języka.

 const openCloseRulesWindow = () => {

To jest utworzenie zmiennej o nazwie openCloseRulesWindow i przypisanie do niej funkcji strzałkowej.
Ale jak zrobisz coś takiego w klasie, to już będzie to znaczyć co innego i będzie to specjalna notacja 'instance field', która jest nową funkcją JSa i jest cukrem składniowym dla przypisania tego w konstruktorze.

6. Mógłbyś powiedzieć mi w jaki sposób oddzielić logikę z GUI? Nie jestem pewny dokładnie o co Ci chodzi - czy o CSSowe zmiany (np. stylów, kolorów), czy o pobieranie elementów z DOM? Na to powinna być jakaś jedna oddzielna klasa/moduł?

To może inaczej - wyobraź sobie, że musisz tę samą grę napisać używając divów, ale jednocześnie musisz tę samą grę napisać, żeby działała w trybie tekstowym.

I teraz tak - pewne rzeczy będą wspólne (logika gry - np. sposób losowania liczb, zasady punktacji itp.), a pewne rzeczy będą związane czysto z GUI (np. wyświetlenie diva na ekranie kontra wypisanie użytkownikowi w konsoli czegoś).

Oczywiście nie musisz robić swojej gry w trybie tekstowym (chociaż czasem się przydają takie rzeczy), bardziej chodzi mi o wyobrażenie sobie sytuacji, że logika gry i GUI to są dwie różne rzeczy.

Np. w klasie Game masz coś takiego:

   this.round = 1;
   this.throwPossibilites = 2;
        
   this.diceThrowBtn = document.querySelector(".player-info__dice-throw");
   this.startGameBtn = document.querySelector(".start-game-window__start-game");

2 pierwsze linijki to logika gry, a kolejne 2 linijki to GUI.
I teraz pomyśl np. że z jakichś powodów musisz wydzielić samą logikę (np. żeby zrobić grę, która będzie działać na serwerze) albo jeśli będziesz chciał zmienić GUI (np. narysować to na canvasie). I mając zaplątane te dwie rzeczy w jednej funkcji(i klasie) nie zrobisz tego.

Tak samo dalej:

 const secondPlayerTotalScore = this.secondPlayerName.textContent !== "Komputer" ?

tutaj wprost traktujesz GUI(w postaci przeglądarkowego DOM i divów) jako miejsce składowania danych, z których wyciągasz nazwę gracza.

I podane przykłady łamią zasadę Single Responsibility Principle, o której również jest mowa w książce Czysty Kod, i która jest zdefiniowana w ten sposób, że klasa powinna mieć tylko jeden powód do zmiany - a tutaj tych powodów jest więcej - bo może będziesz chciał zmienić logikę gry, może sposób wyświetlania na planszy, może sposób zapisywania danych gry (zamiast zapisywania ich w DOM).

@LukeJL:
Jasne, wydaje mi się, że rozumiem obydwie kwestie. Dzięki wielkie za rozjaśnienie, oczywiście wszystkie te rzeczy poprawię.

Ja mam uwagę UX-ową.

To jest strasznie rozwlekłe, napompowane oczywistościami i kompletnie zbędną watą słowną. Prawie nic nie mówi, jak na taką dawkę tekstu.
A to dopiero początek referatu.

Jak użytkownik chce podejrzeć zasady, to się spieszy i potrzebuje zwięzłego komunikatu. Nikt nie chce wtedy studiować esejów z refleksjami w rodzaju "w grze liczą się również umiejętności kalkulacji oraz podejmowania odpowiednich wyborów". Próbuje się tylko wyłowić wzrokiem istotne informacje, a w tym twoim tutorialu jest to frustrujące. Ludziom nie chce się wczytywać w ściany tekstu nawet jak biorą ulotkę z informacjami o leku, więc co dopiero gdy mają ochotę popykać sobie chwilę w kości.

Co do samego kodu, ja tam JavaScriptowcem nie jestem, ale chyba nawet w JS standardem jest - czy powinno być - pisanie testów jednostkowych? W tym repozytorium nie widzę ani jednego...

@V-2: cześć! Dzięki za uwagę UXowa, faktycznie jest to zbędne. Również i to poprawię!

Jeśli chodzi o testy - nie robiłem ich. Aktualnie jestem na etapie nauki, gdzie nie uczyłem się w jaki sposób działają testy, jak się je pisze i raczej na ten moment nie jest to dla mnie priorytet. Wiem że jest coś takiego jak JEST, troszkę o tym poczytałem, ale nie zagłębialem się w tematykę testów za bardzo.