Skip to main content
added 1 character in body
Source Link
m-dev
  • 46
  • 2

#My version

My version

#My version

My version

deleted 29 characters in body
Source Link
m-dev
  • 46
  • 2

Formatting:

Formatting:

Naming of variables:

Naming of variables:

  • e.g. count_1, count_2, count_3 could have been count_X, count_O, count_total e
  • e.g. a1, a2, a3, a4, a5 e
  • e.g. coords does not just store coordinates, also the "mark" of the cell. I called it marks e
  • e.g. record It is not what I would think of as a record. It represents the grid of cells on the board. I called it board_grid. e
  • e.g. isChangePlayer You meant here is O-player. In my version I referred to the players by index: X = 0 and O = 1. I used a property player_on_turn_index on the game object, which is a number. e
  • e.g. isTrue When you start writing isTrue = false, there must be an alarm going of: naming is not logical! :-)

Scoping and objects:

Scoping and objects:

Repetition:

Repetition:

More concise formulations:

More concise formulations:

  • editRecord

editRecord

  • verdictCoords function

verdictCoords

  • isEqual function as mentioned in other comments

isEqual

My version as mentioned in other comments

#My version

Formatting:

Naming of variables:

  • e.g. count_1, count_2, count_3 could have been count_X, count_O, count_total e.g. a1, a2, a3, a4, a5 e.g. coords does not just store coordinates, also the "mark" of the cell. I called it marks e.g. record It is not what I would think of as a record. It represents the grid of cells on the board. I called it board_grid. e.g. isChangePlayer You meant here is O-player. In my version I referred to the players by index: X = 0 and O = 1. I used a property player_on_turn_index on the game object, which is a number. e.g. isTrue When you start writing isTrue = false, there must be an alarm going of: naming is not logical! :-)

Scoping and objects:

Repetition:

More concise formulations:

  • editRecord
  • verdictCoords function
  • isEqual function as mentioned in other comments

My version

Formatting:

Naming of variables:

  • e.g. count_1, count_2, count_3 could have been count_X, count_O, count_total
  • e.g. a1, a2, a3, a4, a5
  • e.g. coords does not just store coordinates, also the "mark" of the cell. I called it marks
  • e.g. record It is not what I would think of as a record. It represents the grid of cells on the board. I called it board_grid.
  • e.g. isChangePlayer You meant here is O-player. In my version I referred to the players by index: X = 0 and O = 1. I used a property player_on_turn_index on the game object, which is a number.
  • e.g. isTrue When you start writing isTrue = false, there must be an alarm going of: naming is not logical! :-)

Scoping and objects:

Repetition:

More concise formulations:

editRecord

verdictCoords

isEqual

as mentioned in other comments

#My version

Source Link
m-dev
  • 46
  • 2

Formatting:

  • Format your code would look nicer. Function or if-blocks will have indentation and you can more easily spot what belongs together. Formatting is a function of your IDE. You might need to put the JavaScript in a separate file (which seems good practice anyway.).

Naming of variables:

  • e.g. count_1, count_2, count_3 could have been count_X, count_O, count_total e.g. a1, a2, a3, a4, a5 e.g. coords does not just store coordinates, also the "mark" of the cell. I called it marks e.g. record It is not what I would think of as a record. It represents the grid of cells on the board. I called it board_grid. e.g. isChangePlayer You meant here is O-player. In my version I referred to the players by index: X = 0 and O = 1. I used a property player_on_turn_index on the game object, which is a number. e.g. isTrue When you start writing isTrue = false, there must be an alarm going of: naming is not logical! :-)

Scoping and objects:

  • You use many global variables that are modified inside global functions. It is usually considered better practice to store state in bundles that belong together as objects with state and methods. You can than access them by storing a limited set of objects as global, you can modify the state inside the methods. This seems easier to understand because you can more easily find which things something belongs and you can better reason about where it could possibly be accessed. It gives you a better general overview of the code.

Repetition:

  • Some of your code was repeted in if and else blocks. e.g. the function cellClicked contains a block for the same action for player 1 and player two. The logic is the same, except for the letter and the color of the mark. It seems more easily to understand if you set the if and else branch for just picking the letter and the color and leave the rest of the code common. It might be better in order not to forget both conditions when you would change something.

More concise formulations:

  • editRecord
function editRecord(x, y, num){ record.forEach(function(item, index){ if(index === y){ item.forEach(function(item, index){ if(x === index){ record[y][x] = num; } }) } }) } 

This function is not needed at all:

record[y][x] = num 

! :-)

  • verdictCoords function
let isMarked = verdictCoords(x, y) function verdictCoords(x, y){ var isTrue = true; coords.forEach(function(item, index){ if(item.x === x && item.y === y){ isTrue = false; } }) return isTrue } 

It should be equal to:

let isMarked = record[y][x] !== 0 
  • isEqual function as mentioned in other comments

My version

Based on your version I wanted to create a revision. But because I thought the structure of your initial version wasn't right, it became a whole new version. It's more how I would have done it. I hope you learn from the different structure. (I now saw you have an update, which I didn't look at for this. It looks a lot cleaner, but I would still do the scoping/object-oriented refactor if you want to do it well.)

// @ts-check // NAMING // - cell: a space on the board // - mark: an x or an o // - winning line: a series of marks that causes a win // - direction: direction a line can be drawn: horizontal, vertical, top-left to bottom-right, bottom-right to top-left // - dimension: horizontal or vertical /** * @typedef { { cell_indices: tCellIndices, player_index: number } } tMark * @typedef { number } tPlayerIndex 0 or 1 (X or O) * @typedef { number[] } tCellIndices [y, x] y- and x-indexes of the cell in the board_grid (note: y first) * @typedef { tCellIndices[] } tLineIndices [start_cell_indices, end_cell_indices] * @typedef { (tPlayerIndex|undefined)[][] } tBoardGrid * An array of arrays in order to access cell content by board_grid[y][x]. * Cell contents is undefined for an empty cell or the index of the player for a cell with a mark. */ // I split up the code in two objects: GamePlay and View // - GamePlay is the logic // - View is the UI // I create a Game Play object in a immediately-invoked function expression in order to limit the scope of variables. // There is only 1 GamePlay object during the use of the website. // The GamePlay object contains the properties/methods that are not related to a specific game object. // The GamePlay object contains a game property, which is the game being played. // During the use of the website multiple games can be played and multiple game objects can be created. const GamePlay = (function create_game_play() { /** The number of marks required to win. */ const MARKS_IN_WINNING_LINE_COUNT = 5 const ROW_COUNT = 15 const COLUMN_COUNT = 15 const CELL_COUNT = ROW_COUNT * COLUMN_COUNT // separate function for typing of ReturnType function create_game() { /** @type {tMark[]} */ let marks = []; /** @type {tBoardGrid} */ let board_grid = [] for (let i = 0; i < COLUMN_COUNT; ++i) { let column = new Array(ROW_COUNT) column.fill(undefined) board_grid.push(column) } /** @param {number[]} cell_indices */ function set_mark(cell_indices) { if (!can_set_mark(cell_indices)) return let player_played_index = game.player_on_turn_index let mark = { cell_indices: cell_indices, player_index: player_played_index, } marks.push(mark) board_grid[cell_indices[0]][cell_indices[1]] = player_played_index let winning_lines = check_victory(mark) let has_won = winning_lines.length > 0 if (has_won) { game.player_won_index = player_played_index } else { game.player_on_turn_index = (player_played_index + 1) % 2 } // Call the view with parts to render. // I pass the game object as well in order to prevent need for accessing the static GamePlay.game variable. View.render_set_mark(game, mark, player_played_index, winning_lines) // Some logic I put inside a separate function inside the set_mark function: // - can_set_mark // - check_victory // Check whether a move is permitted. function can_set_mark(cell_indices) { let is_game_over = game.player_won_index !== undefined if (is_game_over) return false let mark = game.board_grid[cell_indices[0]][cell_indices[1]] let is_cell_unmarked = mark === undefined return is_cell_unmarked } /** * Verify whether the move results in winning lines * @param {tMark} mark * @returns {tLineIndices[]} the lines created by the move. Empty array if no winning move. */ // I changed the logic in order to verify order only the lines that are made by the mark. // One reason was that after you find a winning line, any move would result in a winning line, because 1 was already found. // This code also allows you to discover multiple lines at once and find lines with more than 5 marks. function check_victory(mark) { /** * @param {number[]} direction_step * @returns {tLineIndices|undefined} * Verifies whether the mark is within a winning line for a direction. */ function check_victory_in_direction(direction_step) { /** start and end cell indices */ let start_and_end_cell_indices = [] /** number of marks in the line */ let marks_count = 1 // 1 = this mark for (let backward_or_forward_index = 0; backward_or_forward_index < 2; ++backward_or_forward_index) { // in the first iteration, go in backward direction starting from the mark and find the start of the line: -1 // second iteration, go in forward direction starting from the mark and find the end of the line: 1 let backward_or_forward = backward_or_forward_index === 0 ? -1 : 1 // start or end cell depending on the direction of let start_or_end_cell_indices = mark.cell_indices outer: while (true) { let start_or_end_cell_indices_next = [] for (let i = 0; i < start_or_end_cell_indices.length; ++i) { let direction_dimension_step = backward_or_forward * direction_step[i] let a = start_or_end_cell_indices[i] + direction_dimension_step if (a < 0) break outer start_or_end_cell_indices_next[i] = a } let mark_player_index = game.board_grid[start_or_end_cell_indices_next[0]][start_or_end_cell_indices_next[1]] if (mark_player_index !== mark.player_index) break start_or_end_cell_indices = start_or_end_cell_indices_next ++marks_count } start_and_end_cell_indices.push(start_or_end_cell_indices) } if (marks_count >= MARKS_IN_WINNING_LINE_COUNT) { return start_and_end_cell_indices } } // There are 4 possible directions to form a winning line. let POSSIBLE_WINNING_LINE_DIRECTIONS = [ // notation: [y, x] // [step size in vertical dimension, step size in horizontal dimension] [0, 1], // horizontal [1, 0], // vertical [1, 1], // diagonal top-left to bottom-right [1, -1], // diagonal bottom-left to top-right ] let winning_lines = [] for (let line_direction of POSSIBLE_WINNING_LINE_DIRECTIONS) { let winning_line = check_victory_in_direction(line_direction) if (winning_line) { winning_lines.push(winning_line) } } return winning_lines } } let game = { /** @type {tMark[]} array of marks set in the game (by any of both players) */ marks: marks, /** @type {tBoardGrid} cells by y/column index and then by x/row index */ board_grid: board_grid, player_on_turn_index: 0, /** @type {number|undefined} */ player_won_index: undefined, /** @param {number[]} coords */ set_mark(coords) { set_mark(coords) }, } return game } function start_game() { let game = create_game() game_play.game = game View.render_start_game(game) } let game = create_game() var game_play = { get column_count() { return COLUMN_COUNT }, get row_count() { return ROW_COUNT }, get size() { return [ROW_COUNT, COLUMN_COUNT] }, get cell_count() { return CELL_COUNT }, /** @type {ReturnType<typeof create_game>} game */ game: game, start_game() { start_game() }, } return game_play })(); /** * @typedef {(typeof GamePlay.game)&{}} tGame */ const View = (function create_view() { const CELL_SIZE = 50 // Variables initialized with immediately invoked function expression, in order to get typings right let [el_canvas, ctx, array_el_game_state, el_button_restart] = (function initialize() { const el_canvas = document.getElementById('mycanvas'); if (!el_canvas) throw new Error('no element mycanvas'); if (!(el_canvas instanceof HTMLCanvasElement)) throw new Error('my canvas is not a canvas'); const ctx = el_canvas.getContext("2d"); if (!ctx) throw new Error('canvas has no 2d context') let array_el_game_state = ['el_game_state_1', 'el_game_state_2'].map(id => { let el_game_state = document.getElementById(id) if (!el_game_state) throw new Error('no element ' + id) if (!(el_game_state instanceof HTMLElement)) throw new Error(`element ${id} is not a HTML element`) return el_game_state }) let el_button_restart = document.getElementById('el_button_restart') if (!el_button_restart) throw new Error('no element el_button_restart') if (!(el_button_restart instanceof HTMLButtonElement)) throw new Error() return [el_canvas, ctx, array_el_game_state, el_button_restart] })(); // Initialize canvas en canvas context el_canvas.width = GamePlay.column_count * CELL_SIZE el_canvas.height = GamePlay.row_count * CELL_SIZE // these properties only work when set after el_canvas.width and height ctx.font = "40px Verdana"; ctx.textAlign = "center"; ctx.textBaseline = "middle"; // The next two functions can be called from the game logic object. // I let their names start with render_ /** * @param {tGame} game */ function render_start_game(game) { //game State elements array_el_game_state[0].textContent = "Player 1 made no moves."; array_el_game_state[1].textContent = "Player 2 made no moves."; draw_board_background() for (let y = 0; y < game.board_grid.length; ++y) { for (let x = 0; x < game.board_grid.length; ++x) { draw_cell({ cell_indices: [y, x] }) } } } /** * @param {tGame} game * @param {tMark} mark * @param {tPlayerIndex} player_played_turn_index * @param {tLineIndices[]} winning_lines */ function render_set_mark(game, mark, player_played_turn_index, winning_lines) { draw_mark(mark) draw_winning_lines(player_played_turn_index, winning_lines) draw_game_state_text(game, player_played_turn_index, winning_lines.length > 0) } // The next methods are helper methods of the render methods. // I let theire names start with draw_. const MARK_COLORS_BY_PLAYER_INDEX = ['blue', 'red'] const MARKS_BY_PLAYER_INDEX = ['X', 'O'] /** * @param {{ cell_indices: number[] }} mark */ function draw_cell(mark) { let [y, x] = mark.cell_indices.map(a => a * CELL_SIZE); /* ctx.shadowBlur = 0 ctx.shadowOffsetX = 0 ctx.shadowOffsetY = 0 ctx.fillStyle = 'blue' ctx.fillRect(x, y, CELL_SIZE, CELL_SIZE); */ ctx.fillStyle = 'teal'; ctx.shadowBlur = 4; ctx.shadowColor = '#000'; ctx.shadowOffsetX = 0; ctx.shadowOffsetY = 2; ctx.fillRect(x + 5, y + 5, CELL_SIZE - 10, CELL_SIZE - 10); } /** * Draw a mark. * @param {tMark} mark */ function draw_mark(mark) { let [y, x] = mark.cell_indices.map(a => a * CELL_SIZE); let mark_sign = MARKS_BY_PLAYER_INDEX[mark.player_index] ctx.fillStyle = MARK_COLORS_BY_PLAYER_INDEX[mark.player_index]; ctx.fillText(mark_sign, x + CELL_SIZE * 0.5, y + CELL_SIZE * 0.5); } /** * Draws a new board. Overpaints the old one. */ function draw_board_background() { let size = GamePlay.size.map(a => a * CELL_SIZE) let [width, height] = size // Draw background ctx.beginPath(); ctx.rect(0, 0, width, height); ctx.fillStyle = "rgba(0,122,0, 0.2)"; ctx.strokeStyle = "fuchsia"; ctx.fill(); // Draw grid /* ctx.beginPath(); for (let i = 0; i <= GamePlay.column_count; i++) { ctx.moveTo(i * CELL_SIZE, 0); ctx.lineTo(i * CELL_SIZE, height); } for (let i = 0; i <= GamePlay.row_count; ++i) { ctx.moveTo(0, i * CELL_SIZE); ctx.lineTo(width, i * CELL_SIZE); } ctx.stroke(); */ } /** * Draw the lines through the winning marks. * @param {tPlayerIndex} player_played_turn_index * @param {tLineIndices[]} winning_lines_start_and_end_cell_indices */ function draw_winning_lines(player_played_turn_index, winning_lines_start_and_end_cell_indices) { if (winning_lines_start_and_end_cell_indices.length === 0) return for (let winning_line_start_and_end_cell_indices of winning_lines_start_and_end_cell_indices) { let [start_cell_index, end_cell_index] = winning_line_start_and_end_cell_indices ctx.beginPath(); ctx.moveTo(start_cell_index[1] * CELL_SIZE + CELL_SIZE / 2, start_cell_index[0] * CELL_SIZE + CELL_SIZE / 2); ctx.lineTo((end_cell_index[1] + 1) * CELL_SIZE - CELL_SIZE / 2, (end_cell_index[0] + 1) * CELL_SIZE - CELL_SIZE / 2); let color = MARK_COLORS_BY_PLAYER_INDEX[player_played_turn_index]; ctx.strokeStyle = color; ctx.lineWidth = 11; ctx.stroke(); ctx.closePath(); } } /** * Update the two game state labels above the board. * @param {tGame} game * @param {tPlayerIndex} player_played_turn_index * @param {boolean} is_victory */ function draw_game_state_text(game, player_played_turn_index, is_victory) { let player_number = player_played_turn_index + 1 let moves_count = game.marks.filter(x => x.player_index === player_played_turn_index).length let text if (is_victory) { text = `Player ${player_number} WON in ${moves_count} moves!` } else { text = `Player ${player_number} made ${moves_count} moves.` } array_el_game_state[player_played_turn_index].textContent = text } // Adding event listeners el_canvas.addEventListener("mousedown", function (e) { const x = Math.floor(e.offsetX / CELL_SIZE); /* 0 - 3 on board cell */ const y = Math.floor(e.offsetY / CELL_SIZE); /* 0 - 3 on board cell */ GamePlay.game.set_mark([y, x]) }); el_button_restart.addEventListener('click', function (ev) { GamePlay.start_game() }) return { render_start_game: render_start_game, render_set_mark: render_set_mark, } })(); GamePlay.start_game()
<!DOCTYPE html> <html> <head> <meta name="viewport" content="width=device-width, initial-scale=1.0" /> <title>Gomoku game 5 in a row</title> <style> body { text-align: center; } #canvas { border: 2px solid green; background-color: lightblue; } </style> </head> <body> <h1>Gomoku game 5 in a row. Javascript simple game Canvas.</h1> <p id="el_game_state_1"></p> <p id="el_game_state_2"></p> <canvas id="mycanvas"></canvas> <div> <button id="el_button_restart" type="button">ReStart</button> </div> <script src="behavior.js"></script> </body> </html>