HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Check for wins in a Connect4 variant

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
variantwinsforconnect4check

Problem

Me and my friend are trying to build a Connect4 game with a twist. The twist requires us to have a 7*7 board and to be always able to check all possibilities for a win for both players after each move.

Currently, we are just filling the board with a random distribution of 0,1 and 2 (in the final game it will be 0 for empty, 1 for player 1, 2 for player 2) and then check if there are four of the same kind (even 0) connected, which means someone wins.

What I would like to be interested in:

-
Can this part of the checkForWin function be any cleaner?

if (winRow || winColumn || winDiagonal) {
    if (winRow && !winColumn && !winDiagonal) {
        document.getElementsByClassName("statusArea")[0].innerHTML = "WIN BY ROW";
    }
    if (!winRow && winColumn && !winDiagonal) {
        document.getElementsByClassName("statusArea")[0].innerHTML = "WIN BY COLUMN";
    }
    if (!winRow && !winColumn && winDiagonal) {
        document.getElementsByClassName("statusArea")[0].innerHTML = "WIN BY DIAGONAL";
    }
    if (winRow && winColumn && !winDiagonal) {
        document.getElementsByClassName("statusArea")[0].innerHTML = "WIN BY ROW AND COLUMN";
    }
    if (winRow && !winColumn && winDiagonal) {
        document.getElementsByClassName("statusArea")[0].innerHTML = "WIN BY ROW AND DIAGONAL";
    }
    if (!winRow && winColumn && winDiagonal) {
        document.getElementsByClassName("statusArea")[0].innerHTML = "WIN BY COLUMN AND DIAGONAL";
    }
    if (winRow && winColumn && winDiagonal) {
        document.getElementsByClassName("statusArea")[0].innerHTML = "WIN BY ROW, COLUMN AND DIAGONAL";
    }
}


-
Some testing to check if the checkForWin function really always does what we want it to. I tried a lot of random distributions, but I'm just not sure if everything works out the right way.

  • I really had to mess around with the CSS until it looked like it does now. Please help me to clean this up.



  • General comments, style advice and hints f

Solution

I recommend separating your logic from UI by introducing a function getWinMessage and assigning the innerHTML or even better textContent later on:

function getWinMessage(winRow, winColumn, winDiagonal) {
  if ( winRow && !winColumn && !winDiagonal) return "WIN BY ROW";
  if (!winRow &&  winColumn && !winDiagonal) return "WIN BY COLUMN";
  if (!winRow && !winColumn &&  winDiagonal) return "WIN BY DIAGONAL";
  if ( winRow &&  winColumn && !winDiagonal) return "WIN BY ROW AND COLUMN";
  if ( winRow && !winColumn &&  winDiagonal) return "WIN BY ROW AND DIAGONAL";
  if (!winRow &&  winColumn &&  winDiagonal) return "WIN BY COLUMN AND DIAGONAL";
  if ( winRow &&  winColumn &&  winDiagonal) return "WIN BY ROW, COLUMN AND DIAGONAL";
}

if (winRow || winColumn || winDiagonal) {
  document.getElementsByClassName("statusArea")[0].textContent = getWinMessage(winRow, winColumn, winDiagonal);
}


For brevity, you could encode your messages in a three dimensional array with the dimensions representing row, column and diagonal (no-win = 0, win = 1). This is shorter but slightly less readable:

const winMessages = [[["", "WIN BY DIAGONAL"], ["WIN BY COLUMN", "WIN BY COLUMN AND DIAGONAL"]], [["WIN BY ROW", "WIN BY ROW AND DIAGONAL"], ["WIN BY ROW AND COLUMN", "WIN BY ROW, COLUMN AND DIAGONAL"]]];

function getWinMessage(winRow, winColumn, winDiagonal) {
  return winMessages[+winRow][+winColumn][+winDiagonal];
}


Regarding CSS and HTML: Please add those to your question, e.g. as a stack snippet.

Code Snippets

function getWinMessage(winRow, winColumn, winDiagonal) {
  if ( winRow && !winColumn && !winDiagonal) return "WIN BY ROW";
  if (!winRow &&  winColumn && !winDiagonal) return "WIN BY COLUMN";
  if (!winRow && !winColumn &&  winDiagonal) return "WIN BY DIAGONAL";
  if ( winRow &&  winColumn && !winDiagonal) return "WIN BY ROW AND COLUMN";
  if ( winRow && !winColumn &&  winDiagonal) return "WIN BY ROW AND DIAGONAL";
  if (!winRow &&  winColumn &&  winDiagonal) return "WIN BY COLUMN AND DIAGONAL";
  if ( winRow &&  winColumn &&  winDiagonal) return "WIN BY ROW, COLUMN AND DIAGONAL";
}

if (winRow || winColumn || winDiagonal) {
  document.getElementsByClassName("statusArea")[0].textContent = getWinMessage(winRow, winColumn, winDiagonal);
}
const winMessages = [[["", "WIN BY DIAGONAL"], ["WIN BY COLUMN", "WIN BY COLUMN AND DIAGONAL"]], [["WIN BY ROW", "WIN BY ROW AND DIAGONAL"], ["WIN BY ROW AND COLUMN", "WIN BY ROW, COLUMN AND DIAGONAL"]]];

function getWinMessage(winRow, winColumn, winDiagonal) {
  return winMessages[+winRow][+winColumn][+winDiagonal];
}

Context

StackExchange Code Review Q#160783, answer score: 4

Revisions (0)

No revisions yet.