Proszę o przejrzenie mojego kodu React i wytknięcie błędów.

Witam,
zaczynam dopiero z React-em i chciałbym od razu zacząć uczyć się dobrych praktyk podczas pisania kodu. Napisałem prostą grę w kółko i krzyżyk. Proszę o rzucenie okiem na kod oraz o powiedzenie mi czy jest dobrze napisany, jak mógłbym go zoptymalizować, czy nie popełniam błędów w nim i ogólnie chyba wiadomo o co chodzi.

Link do Codesandbox:
https://codesandbox.io/s/hungry-shape-n8oo0m

Testy nie przechodzą :) Poza tym cała logika gry jest w jednym pliku, ja bym wszystko rozbił, osobno mechanika, osobno AI.

@bakunet

bakunet napisał(a):

Testy nie przechodzą :)

Kolega jeszcze nie tworzył testów :) Ten plik App.test.js pochodzi z gotowej templatki i pojawia się automatycznie po skonfigurowaniu projektu.

@Gouda105
To co teraz napiszę nie jest "jako tako" błędem, bo dużo starszych aplikacji zostało tak napisanych, ale obecnie React coraz bardziej rezygnuje już z komponentów klasowych i idzie w kierunku komponentów funkcyjnych, hooków i lepszego wsparcia TypeScriptu.

https://pl.reactjs.org/docs/hooks-intro.html

W dokumentacji można znaleźć takie porównanie

Nowy komponent funkcyjny

import React, { useState } from 'react';

function Example() {
  const [count, setCount] = useState(0);

  return (
    <div>
      <p>Naciśnięto {count} razy</p>
      <button onClick={() => setCount(count + 1)}>
        Naciśnij mnie
      </button>
    </div>
  );
}

vs starsza wersja w formie klasy

import React from 'react';

class Example extends React.Component {
  constructor(props) {
    super(props);
    this.state = {
      count: 0
    };
  }

  render() {
    return (
      <div>
        <p>Kliknięto {this.state.count} razy</p>
        <button onClick={() => this.setState({ count: this.state.count + 1 })}>
          Kliknij mnie
        </button>
      </div>
    );
  }
}

I po skończeniu obecnego tutoriala najlepiej byłoby poszukać jakiegoś innego źródła, które omawia nowszy zapis.
Pomiędzy dwoma wersjami jest sporo różnic, ale jeśli zrozumiałeś obecny materiał, który zastosowałeś w swoim przykładzie to myślę, że nie będziesz miał z tym problemu.

1. AI jest głupie. Mogę wstawić 2 krzyżyki w jednej linii i komputer zamiast mnie przyblokować, to stawia w randomowym miejscu. I odwrotnie - jeśli to komputer ma dwa kółka w jednej linii, to zamiast postawić trzecie kółko i wygrać, to stawia w randomowym miejscu.

2. w handlerze eventu odwołujesz się bezpośrednio do DOM, żeby wydostać index: e.target.getAttribute("index"). Tylko, że jest to zbyteczne i w zasadzie omijasz Reacta i uzależniasz się od prawdziwego DOMu bez wyraźnej potrzeby. W React zwykle tak się robi, że przekazujesz dane do handlerów eventów, czyli w komponencie Board zamiast:

onClick={this.onPlayerMove}

mógłbyś zrobić tak:

onClick={() => this.onPlayerMove(i)}

czyli przekazujesz od razu te dane, których rzeczywiście potrzebujesz (a obiektu eventu e w ogóle nie potrzebujesz w tym przypadku)

3. czemu bindujesz handlera zdarzeń do this w komponencie Board?

this.onPlayerMove = props.onPlayerMove.bind(this);

wygląda to na błąd/pomyłkę albo, jeśli to celowe, to dość partyzancki kod, jeśli chcesz pożyczać this w ten sposób handlerowi zdarzenia.

4. zamiast inicjalizować tablicę w ten sposób:

board: [null, null, null, null, null, null, null, null, null],

możesz utworzyć tablicę z 9 pustymi elementami i użyć metody fill:

board: new Array(9).fill(null),

5. zamiast kombinować jak koń pod górę

    let indexes = this.state.board.reduce(function (a, e, i) {
      if (e === null) a.push(i);
      return a;
    }, []);

możesz użyć metody filter:

let indexes = this.state.board.filter(item => item === null);

6. w komponencie Game masz taki kod:

    winMap.forEach((element) => {
      if (...) {
        (...)
        return;
      }
      });

to return jest zbędne, bo i tak to koniec funkcji. Tylko nie wiem, jaki był twój zamiar. Bo to trochę wygląda, jakbyś chciał wyjść z funkcji wyżej (a w ten sposób nie wyjdziesz, bo return wychodzi tylko z bezpośredniej funkcji).