week 1 homework

[lanbau]

@cbas

[sebdeckers]

Code Review

Comments

  // fix click outside grid bug
  if (tile.className !== 'tile') return
  // fix stealing grids
  if (tile.textContent) return

Smells

• Comments often indicate the code is not clear to read. Use meaningfully named variables (or functions) to "label" the statement.
• Repeated return statement. Refactor to avoid duplicated statements.

Refactor

function isValidClickTarget (tile) { return isTile(tile) && isEmpty(tile) }
function isTile (tile) { return tile.className !== 'tile' }
function isEmpty(tile) { return !!tile.textContent }
// ...
if (!isValidClickTarget(tile)) return

Code Duplication

  if (currentPlayer === 'one') {
    tile.textContent = 'x'
    currentPlayer = 'two'
  } else {
    tile.textContent = 'o'
    currentPlayer = 'one'
  }

Smells

• Duplicated lines with little variation.

Refactor

var currentPlayerSymbol = currentPlayer === 'one' ? 'x' : 'o'
var nextPlayerName = currentPlayer === 'one' ? 'two' : 'one'
tile.textContent = currentPlayerSymbol
currentPlayer = nextPlayerName

Duplicate Code

var tile11 = document.querySelector('.row:nth-of-type(1) .tile:nth-of-type(1)').textContent
// ...

Smells

• Looks too similar.

Refactor

function getTile (x, y) {
  var rowSelector = '.row:nth-of-type(' + x + ')'
  var columnSelector = '.tile:nth-of-type(' + y + ')'
  var tileSelector = rowSelector + ' ' + columnSelector
  return document.querySelector(tileSelector).textContent
}
var tile11 = getTile(1, 1)
// ... repeat

Then continue to refactor any repeated similar code.

More Repetition

  var rowx1 = (tile11 === 'x' && tile12 === 'x' && tile13 === 'x')
  var rowx2 = (tile21 === 'x' && tile22 === 'x' && tile23 === 'x')
  var rowx3 = (tile31 === 'x' && tile32 === 'x' && tile33 === 'x')
  if (rowx1 || rowx2 || rowx3) {
    console.log('Player X Wins!')
    document.getElementById('score').innerHTML = 'Player X Wins!'
    var ans = prompt('Type yes To Play Again')
    if (ans === 'yes') { window.location.reload() }
  }

Smells

• This is also repeated over and over.

Refactor

• Look for the underlying pattern that results in the repetition. In this case there are the two players and the various winning states. Refactor to two nested loops, one for the players and one for the winning states. That should result in non-repeated, factored code. Exercise left to the reader. :stuck_out_tongue_winking_eye: