patternjavascriptMinor
Simple Tic-Tac-Toe App Using Knockout.JS
Viewed 0 times
simpleapptoeticknockouttacusing
Problem
This is really the first full app I've written in JavaScript minus CSS styling which I plan on adding later.
Are there any ways it can be improved? Any ways I can make it cleaner or easier to read?
`"use strict";
// Constructor function for individual parts of game grid square
// Each grid square contains a state property of empty, x, or o.
// Also contains row and column properties
function GridSquare(column, row) {
this.state = ko.observable('n');
// Not an observable because location will never change
this.row = row;
this.column = column;
}
// Constructor for game board object
// Contains a list of grid squares and gives each grid square a row and column
// Also contains state property which determines if game is currently playing
function GameBoard() {
this.topRow = ko.observableArray(
[new GridSquare('leftColumn', 'topRow'),
new GridSquare('middleColumn', 'topRow'),
new GridSquare('rightColumn', 'topRow')]);
this.middleRow = ko.observableArray(
[new GridSquare('leftColumn', 'middleRow'),
new GridSquare('middleColumn', 'middleRow'),
new GridSquare('rightColumn', 'middleRow')]);
this.bottomRow = ko.observableArray(
[new GridSquare('leftColumn', 'bottomRow'),
new GridSquare('middleColumn', 'bottomRow'),
new GridSquare('rightColumn', 'bottomRow')]);
// The number of moves made so far
this.moves = 0;
// State property to determine turns and whether or not the game is finished
// Initial state is xturn
// States include:
// xturn for X's turn
// oturn for O's turn
// draw for when a draw occurs
// xwins for when X wins the game
// owins for when O wins the game
this.state = ko.observable('xturn');
// Computed observable to determine the turn or if the game is finished
this.turn = ko.computed( function() {
var turn,
state = this.state();
switch ( st
Are there any ways it can be improved? Any ways I can make it cleaner or easier to read?
`"use strict";
// Constructor function for individual parts of game grid square
// Each grid square contains a state property of empty, x, or o.
// Also contains row and column properties
function GridSquare(column, row) {
this.state = ko.observable('n');
// Not an observable because location will never change
this.row = row;
this.column = column;
}
// Constructor for game board object
// Contains a list of grid squares and gives each grid square a row and column
// Also contains state property which determines if game is currently playing
function GameBoard() {
this.topRow = ko.observableArray(
[new GridSquare('leftColumn', 'topRow'),
new GridSquare('middleColumn', 'topRow'),
new GridSquare('rightColumn', 'topRow')]);
this.middleRow = ko.observableArray(
[new GridSquare('leftColumn', 'middleRow'),
new GridSquare('middleColumn', 'middleRow'),
new GridSquare('rightColumn', 'middleRow')]);
this.bottomRow = ko.observableArray(
[new GridSquare('leftColumn', 'bottomRow'),
new GridSquare('middleColumn', 'bottomRow'),
new GridSquare('rightColumn', 'bottomRow')]);
// The number of moves made so far
this.moves = 0;
// State property to determine turns and whether or not the game is finished
// Initial state is xturn
// States include:
// xturn for X's turn
// oturn for O's turn
// draw for when a draw occurs
// xwins for when X wins the game
// owins for when O wins the game
this.state = ko.observable('xturn');
// Computed observable to determine the turn or if the game is finished
this.turn = ko.computed( function() {
var turn,
state = this.state();
switch ( st
Solution
On surface it's a well laid out code with bunch of helpful comments and good method names.
However, looking deeper the data model of this game looks rather odd. Instead of storing the game state in a simple 2D array as such:
Each cell is instead indexed by named coordinates like
Another big oddity is that in addition to keeping the gameboard state in the grid where it naturally belongs, it's also partly kept in each player object:
Each of these arrays in
However, instead of just looping through all these arrays to determine the winner, some kind of very convoluted logic is applied, which only checks the row, column and diagonal where the last move was made. Maybe an optimization, but completely needless for such a small game.
But really... you should just store your data in a simple 3x3 array and inspect this array after each turn to determine the winner. And try to think of how would you build a tic-tac-toe of 100x100 grid.
However, looking deeper the data model of this game looks rather odd. Instead of storing the game state in a simple 2D array as such:
var grid = [
[0, 0, 0],
[0, 0, 0],
[0, 0, 0],
];Each cell is instead indexed by named coordinates like
("middleColumn", "topRow"). This completely hard-codes this solution to 3x3 grid. Attempting to extend this solution into a larger grid would lead to a lot of trouble.Another big oddity is that in addition to keeping the gameboard state in the grid where it naturally belongs, it's also partly kept in each player object:
function Player(name) {
this.name = name;
this.squares = {
topRow: [],
middleRow: [],
bottomRow: [],
leftColumn: [],
middleColumn: [],
rightColumn: [],
leftDiagonal: [],
rightDiagonal: []
};
}Each of these arrays in
Player.squares keeps a count of how many winning points a player has already scored in rows, columns or diagonals. These arrays are checked after each turn to determine if player has won - first player to get 3 marks into any of these arrays wins.However, instead of just looping through all these arrays to determine the winner, some kind of very convoluted logic is applied, which only checks the row, column and diagonal where the last move was made. Maybe an optimization, but completely needless for such a small game.
But really... you should just store your data in a simple 3x3 array and inspect this array after each turn to determine the winner. And try to think of how would you build a tic-tac-toe of 100x100 grid.
Code Snippets
var grid = [
[0, 0, 0],
[0, 0, 0],
[0, 0, 0],
];function Player(name) {
this.name = name;
this.squares = {
topRow: [],
middleRow: [],
bottomRow: [],
leftColumn: [],
middleColumn: [],
rightColumn: [],
leftDiagonal: [],
rightDiagonal: []
};
}Context
StackExchange Code Review Q#91451, answer score: 3
Revisions (0)
No revisions yet.