Chomp game in javascript

Question

I tried to make a 2-player Chomp game in javascript:

const num_rows = 4;
const num_cols = 7;
var cur_player = 1;
var rows = [];
var buttons = [];
cur_player_text = document.createElement('div');
cur_player_text.innerText = "Current player: " + cur_player;
cur_player_text.id = "cur_player";
document.body.appendChild(cur_player_text);
for(var i = num_rows; i>0; i--) {
 rows[i] = document.createElement('div');
 rows[i].id = "row" + [i];
 document.body.appendChild(rows[i]);
 buttons[i] = [];
 for(var j = 1; j<=num_cols; j++) {
 buttons[i][j] = document.createElement('button');
 buttons[i][j].className = "chocolate";
 buttons[i][j].id = "button" + [i] + "," + [j];
 buttons[i][j].innerText = "x";
 buttons[i][j].setAttribute('onclick', 'squareClick(' + i + ', ' + j + ');');
 rows[i].appendChild(buttons[i][j]);
 }
}
buttons[1][1].setAttribute('disabled', '');
function squareClick(row, col) {
 var i = row;
 var j = col;
 while(buttons[i] != undefined && buttons[i][j] != undefined && buttons[i][j].getAttribute('disabled') == null) {
 while(buttons[i][j] != undefined && buttons[i][j].getAttribute('disabled') == null) {
 buttons[i][j].setAttribute('disabled', '');
 j++;
 }
 row = row+1;
 i = row;
 j = col;
 }
 if(buttons[1][2].getAttribute('disabled') == '' && buttons[2][1].getAttribute('disabled') == '') {
 document.getElementById('cur_player').innerText = "Winner: " + cur_player;
 }
 else {
 cur_player = cur_player%2 + 1;
 document.getElementById('cur_player').innerText = "Current player: " + cur_player;
 }
}

It seems to work but the code is very ugly. How can it be made cleaner and more elegant?

Answer 1

Here are my design and style suggestions:

• Format your code with Prettier and use a linter.
  • Be consistent with single or double quotes (Prettier will fix this).
  • if( should be if ( (Prettier will fix this).
  • Avoid lines longer than 70 chars or so (Prettier will fix this).
• Use const for all variables except for loop counters, use let (ESLint will fix this, along with most of the below).
• Prefer for over while loops.
• Never use == and !=. Always === and !== to avoid surprising type coercion bugs.
• Use camelCase, not snake_case for JS.
• Use kebab-case instead of snake_case for CSS.
• Don't dynamically generate ids. Use classes and indices or data attributes.
• Avoid storing and modifying state in the DOM. Keep your own data structures so you can ensure one-directional data flow to avoid bugs (reading/writing DOM properties comes with many gotchas and pushes the data into places outside of the module's private control).
• Instead of onclick, use addEventListener.
• Use row++; instead of row = row+1;.
• Use template literals rather than string concatenation for things like "button" + [i] + "," + [j];.
• Move complex conditions to functions.
• Break code into logical functions; consider a class, closure or module to collect the loose variables and functions into a unit.
• Avoid coding in the global scope.
• Use zero-indexing.

Suggested rewrite (I'm storing disabled state in the DOM, breaking my own rule--next refactor I'd move that into objects):

const Chomp = ({container, rows, cols}) => {
 // TODO throw if rows or cols are < 3 or container isn't an element
 const statusEl = document.createElement("div");
 const gridEl = document.createElement("div");
 container.append(statusEl, gridEl);
 const grid = [...Array(rows)].map(() => [...Array(cols)]);
 let currentPlayer;
 const isWon = () =>
 grid[rows-1][1].getAttribute("disabled") &&
 grid[rows-2][0].getAttribute("disabled");
 const updateStatus = () => {
 statusEl.innerText = isWon() ?
 `Winner: ${currentPlayer}` :
 `Current player: ${currentPlayer}`;
 };
 const move = (x, y) => {
 for (let yy = 0; yy <= y; yy++) {
 for (let xx = x; xx < grid[yy].length; xx++) {
 if (grid[yy][xx].getAttribute("disabled")) {
 break;
 }
 grid[yy][xx].setAttribute("disabled", true);
 }
 }
 currentPlayer = currentPlayer % 2 + 1;
 };
 const makeButton = (x, y) => {
 const btn = document.createElement("button");
 btn.textContent = "x";
 btn.addEventListener("click", () => {
 move(x, y);
 updateStatus();
 });
 grid[y][x] = btn;
 return btn;
 };
 const makeRow = y => {
 const row = document.createElement("div");
 for (let x = 0; x < cols; x++) {
 row.appendChild(makeButton(x, y));
 }
 return row;
 };
 const makeGrid = () => {
 for (let y = 0; y < rows; y++) {
 gridEl.appendChild(makeRow(y));
 }
 grid[rows-1][0].setAttribute("disabled", true);
 };
 return {
 play() {
 gridEl.innerHTML = "";
 currentPlayer = 1;
 makeGrid();
 updateStatus();
 }
 };
};
(() => {
 const container = document.querySelector(".chomp");
 const chomp = Chomp({rows: 4, cols: 7, container});
 chomp.play();
})();

<div class="chomp"></div>