weejh / wdi-sg-playground

Homework and project - 2015
ISC License
1 stars 0 forks source link

tic tac toe for review. thanks. #2

Open weejh opened 8 years ago

weejh commented 8 years ago

@cbas, @sevresbabylone

sebdeckers commented 8 years ago

:+1: Nicely done! :star:

Element-specific event handlers

Split the long event listener function into multiple functions that deal with just one element.

document.querySelector('.button1')
  .addEventListener('click', event => {
    var button = event.target
    // ... button click code
    reSet()
    document.querySelector('.status').textContent = 'It is X\'s turn'
    currentPlayer = 'one'
  })
Array.from(document.querySelectorAll('.tile')).forEach(tile =>
  tile.addEventListener('click', () => {
    if (tile.textContent) return
    // ... tile click code
    // ...
  })

Avoid setting too much CSS in JS

Use classes to group a bunch of CSS declarations into a single statement.

.tile {
  color: black;
  font-size: 160;
}
.tile.one { background-color: red; }
.tile.two { background-color: purple; }
  var playerSymbol = currentPlayer === 'one' ? 'x' : 'o'
  tile.classList.add(currentPlayer)
  currentPlayer = currentPlayer === 'one' ? 'two' : 'one'
  document.querySelector('.status').textContent = 'It is ' +
    playerSymbol.toUpperCase() +
    '\'s turn'

Code Duplication

A lot of code looks repeated identically except for a few values. See other TTT code reviews for how to solve that.

weejh commented 8 years ago

Thanks for your review.