7
\$\begingroup\$

I'm relatively new to react and attempted to create a simple Tic-Tac-Toe game. Would be great if i could get some feedback on this. Again, at this point using only React (no Redux).

main.js - main file which adds our App component to the DOM

import React from 'react';
import ReactDOM from 'react-dom';
import App from './container/App';

document.addEventListener('DOMContentLoaded', function() {
  ReactDOM.render(
    React.createElement(App),
    document.getElementById('mount')
  );
});

App.js - root component that contains the Board and Dashboard. One specific question I had here was: when I receive the reset() event from my Dashboard App, what's the best practice on how I can end up triggering the reset() of Board?

import React, { Component } from 'react';
import Board from './Board'
import Dashboard from './Dashboard'

export default class App extends Component {

    constructor(props) {
        super(props)

        this.state = {
            winner: ''
        }
    }

    reset() {
        console.log('how do i pass reset to Board...?')

    }

    hasWinner(winner) {
        this.setState({
            winner: winner
        })
    }

    render() {
        return (
            <div>
                <Board rows={3} cols={3} hasWinner={this.hasWinner.bind(this)} />
                <Dashboard winner={this.state.winner} reset={this.reset.bind(this)} />
            </div>
        )
    }

}

Board.js - Board component that contains Cell components! Please ignore the checkWinner() method for the purpose of this review.

import React, { Component } from 'react';
import Cell from './Cell'
import styles from '../css/board.css'

export default class Board extends Component {

    constructor(props) {
        super(props)

        let board = this.createBoard();

        this.state = {
            board: board,
            currentPlayer: 'X'
        }
    }

    createBoard() {
        let board = new Array(this.props.rows);
        for ( var row = 0 ; row < this.props.rows ; row++ ) {
            board[row] = new Array(this.props.cols);
            board[row].fill('');
        };  

        return board;   
    }

    checkWinner() {
        // TODO: please add correct winner algo!
        return this.state.board[0][0];
    }

    drawBoard() {
        let board = [];
        for ( let i = 0 ; i < this.props.rows ; i++ ) {
            for ( let j = 0 ; j < this.props.cols ; j++ ) {
                var id = i + '' + j;
                board.push(
                    <Cell 
                        key={id}
                        id={id}
                        play={this.play.bind(this)}
                        value={this.state.board[i][j]} />
                );
            }
        }
        return board;
    }

    reset() {
        let board = this.createBoard();
        this.setState({
            board: board,
            currentPlayer: 'X'
        })
    }

    play(ij) {
        let i = ij[0];
        let j = ij[1];
        let board = this.state.board;
        board[i][j] = this.state.currentPlayer;
        this.setState({
            board: board,
            currentPlayer: this.state.currentPlayer==='X'?'O':'X'
        })

        let winner = this.checkWinner();
        if ( winner != '' ) {
            this.props.hasWinner(winner);
            this.reset();
        }
    }

    getClassName() {
        return styles.board
    }

    render() {
        return (
            <div className={this.getClassName()}>
                {this.drawBoard()}
            </div>
        )
    }

}

Cell.js - Cell component

import React, { Component } from 'react';
import styles from '../css/cell.css'

export default class Cell extends Component {

    constructor(props) {
        super(props)
    }

    getClassName() {
        return styles.emptycell
    }

    play() {
        if ( this.props.value === '' )
            this.props.play(this.props.id)
    }

    render() {
        return (
            <div 
                onClick={this.play.bind(this)} 
                className={this.getClassName()}>
                    {this.props.value}
            </div>
        )
    }

}

Dashboard.js - Component to show winner and contains a reset button.

import React, { Component } from 'react'

export default class Dashboard extends Component {  

    constructor(props) {
        super(props)

    }

    render() {
        const winner = this.props.winner
                            ?<h1>And the winner is: {this.props.winner}</h1>
                            :<p></p>
        return (
            <div>
                {winner}                    
                <input type="button" value="reset" onClick={this.props.reset} />
            </div>
        )

    }
}
\$\endgroup\$
  • \$\begingroup\$ Welcome to Code Review! Great job formatting your question - hopefully you get some helpful feedback! \$\endgroup\$ – avojak Jan 15 '17 at 5:26
  • \$\begingroup\$ ^ Thanks and looking forward to some helpful pointers. \$\endgroup\$ – nev.m Jan 15 '17 at 5:47
2
\$\begingroup\$

You have two options for handling the reset event.

  1. If the Board component absolutely must handle its own logic for setting its initial board state, you can add a ref prop to it, which allows you to hold onto a reference to that instance of the component, and call methods on that instance:

    reset() {
      this.board.reset();
    }
    ...
    render() {
      return (
        <div>
          <Board rows={3}
                 cols={3}
                 hasWinner={this.hasWinner.bind(this)}
                 ref={(board) => { this.board = board; }} />
          <Dashboard winner={this.state.winner} reset={this.reset.bind(this)} />
        </div>
      )
    }
    
  2. Your second (and preferred) option is to "lift up" the board state from the Board component. This is the approach that is recommended over using a ref in the React documentation. Essentially, you want to move your board state and the createBoard method from the Board component to the App component.


A note on naming:

Your Board component's hasWinner prop sounds like it should be receiving a boolean value, rather than a callback. A common convention in React is to prefix callback props with "on," so your prop names could become onWin, onPlay, and onReset, for example.


Finally, in your Dashboard component, instead of rendering an empty p tag when there is no winner, you can take advantage of the fact that React skips rendering values like false and null. So you can replace your ternary statement with something simpler:

const winner = this.props.winner && <h1>And the winner is: {this.props.winner}</h1>
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.