8
\$\begingroup\$

I know it is a simple game, but what I am doing here is practice what I took in an AI course. I'm also playing a little with the canvas and trying to improve the readability of my code and using JavaScript as an object oriented language.

I divided my code into 3 main classes (objects).

  1. Board class which represents the board object and contain its properties like width and height and the canvas to draw with.
  2. Cell class which represents the individual cell object and contain its properties also.

  3. The app script. This is not a class, it is just a the script that makes use of the other scripts and runs the game.

    • Is what I'm doing by dividing my code into these classes reasonable or it can be done in a simpler way?
    • Is making these classes affecting the performance or not?
    • Does my work need to be divided into more classes(objects)?
    • What I can do better?

Note: I have not implemented the minimax algorithm yet.

index.html

<html>
 <head>
 <title>Tic</title>
 <script src="js/jquery-2.1.3.min.js"></script>
 <script src="js/Board.js"></script>
 <script src="js/Cell.js"></script>
 <script src="js/app.js"></script>
 <link type="text/css" rel="stylesheet" href="css/bootstrap.min.css"/>
 <link type="text/css" rel="stylesheet" href="css/style.css"/>
 </head>
 <body>
 <div class="container">
 <div class="row" style="text-align: center">
 <div class="col-md-6 col-md-offset-3">
 <div class="pin">
 <h1>Tic Tac Toc</h1>
 </div>
 </div>
 </div>
 <div class="row" style="text-align: center">
 <div class="col-md-6 col-md-offset-3">
 <div class="pin" id="content">
 <canvas id="canvas" width="400" height="400"></canvas>
 </div>
 </div>
 </div>
 </div>
 </body>
</html>

app.js

$(document).ready(function(){
 var canvas = $("#canvas").get(0);
 var ctx = canvas.getContext('2d');
 var data = 
 {
 canvas : canvas,
 ctx : ctx,
 x : canvas.width/2-150,
 y : canvas.height/2-150,
 width : 300,
 height : 300,
 playerX: "Abdulaziz",
 playerY: "salmaaa"
 };
 var board = new Board(data);
 board.drawBoard();
 $("#canvas").click({board: board, canvasId: "canvas"}, board.click);
});

Board.js

function Board(data){
 data = (data === 'undefined') ? {} : data;
 if(data){
 this.canvas = data.canvas;
 this.ctx = data.ctx;
 this.cell = [];
 this.isXTurn = true;
 this.gameStatus = "turn";
 this.moves = 0;
 this.winningCombinations = 
 [
 [{x:0,y:0},{x:1,y:0},{x:2,y:0}],[{x:0,y:1},{x:1,y:1},{x:2,y:1}],[{x:0,y:2},{x:1,y:2},{x:2,y:2}],
 [{x:0,y:0},{x:0,y:1},{x:0,y:2}],[{x:1,y:0},{x:1,y:1},{x:1,y:2}],[{x:1,y:0},{x:1,y:1},{x:1,y:2}],
 [{x:0,y:0},{x:1,y:1},{x:2,y:2}],[{x:2,y:0},{x:1,y:1},{x:0,y:2}]
 ];
 if(data.x){
 this.x = data.x;
 this.y = data.y;
 this.width = data.width;
 this.height = data.height;
 this.cellWidth = this.width/3;
 this.cellHeight = this.height/3;
 this.playerX = data.playerX;
 this.playerY = data.playerY;
 }else{
 this.width = 300;
 this.height = 300;
 this.x = this.canvas.width/2-this.width/2;
 this.y = this.canvas.height/2-this.height/2;
 this.cellWidth = this.width/3;
 this.cellHeight = this.height/3;
 this.playerX = "Player X";
 this.playerY = "Player Y";
 }
 for(var i=0 ; i<3 ; i++){
 this.cell.push([]);
 for(var j=0 ; j<3 ; j++){
 var data = {
 x: j*this.cellWidth,
 y: i*this.cellHeight,
 width: this.cellWidth,
 height: this.cellHeight,
 canvas: this.canvas,
 ctx: this.ctx
 };
 var cell = new Cell(data);
 this.cell[i].push(cell);
 }
 } 
 }else{
 //undefined data object
 }
}
Board.prototype.drawBoard = function(){
 //clear area to be drawn upon
 this.ctx.clearRect(this.x, this.y, this.width, this.height);
 //choose color of the stroke of the board and then drawing it
 this.ctx.fillStyle = "#000000";
 this.ctx.strokeRect(this.x, this.y, this.width, this.height);
 //draw the lines that defines the cells of the board
 this.ctx.beginPath();
 //left verticale line
 this.ctx.moveTo((this.x+this.width/3), this.y);
 this.ctx.lineTo((this.x+this.width/3), (this.y+this.height));
 //right verticale line
 this.ctx.moveTo((this.x+2*this.width/3), this.y);
 this.ctx.lineTo((this.x+2*this.width/3), (this.y+this.height));
 //upper horizontal line
 this.ctx.moveTo(this.x, (this.y+this.height/3));
 this.ctx.lineTo((this.x+this.width), (this.y+this.height/3));
 //bottom horizontal line
 this.ctx.moveTo(this.x, (this.y+2*this.height/3));
 this.ctx.lineTo((this.x+this.width), (this.y+2*this.height/3));
 //begin stroking the path then release it
 this.ctx.stroke();
 this.ctx.closePath();
};
Board.prototype.getCell = function(coord){
 var cHor = (coord.mouseX - coord.boardX)/this.cell[0][0].width;
 var cVer = (coord.mouseY - coord.boardY)/this.cell[0][0].height;
 return {h : parseInt(cHor), v: parseInt(cVer)};
};
Board.prototype.getCellCoord = function(cHor, cVer){
 var cX = cHor * this.width/3;
 var cY = cVer * this.height/3;
 return {x : cX, y: cY};
};
Board.isInBounds = function(coord){
 return (coord.mouseX > coord.boardX && coord.mouseX < coord.boardX+coord.width) && 
 (coord.mouseY > coord.boardY && coord.mouseY < coord.boardY+coord.height);
};
Board.isWinCombo = function(combo, board){
 return (board.cell[combo[0].x][combo[0].y].player === board.cell[combo[1].x][combo[1].y].player) &&
 (board.cell[combo[1].x][combo[1].y].player === board.cell[combo[2].x][combo[2].y].player) &&
 (board.cell[combo[0].x][combo[0].y].player !== "");
};
Board.prototype.checkStatus = function(board){
 if(board.moves === 9) return "tie";
 for(var i=0 ; i<this.winningCombinations.length ; i++){
 var combo = this.winningCombinations[i];
 if(Board.isWinCombo(combo, board)) return "win";
 }
 return "turn";
};
Board.prototype.drawStatusBar = function(board, message){
 board.ctx.clearRect(board.x, board.y+board.height+10, board.width, board.height/10);
 board.ctx.strokeRect(board.x, board.y+board.height+10, board.width, board.height/10);
 board.ctx.font = "20px serif";
 board.ctx.fillText(message, board.x+5, board.y+board.height+30);
};
Board.prototype.click = function(e){
 //this a callback function so this identifier refers to 
 //the canvas object that the event listener is attached to
 //not the board object
 var board = e.data.board;
 //e.originalEvent.layerX returns the position of the mouse relative to the canvas not the page or the screen
 //board.x returns the position of the board inside the canvas
 var coord = {
 mouseX: e.originalEvent.layerX,
 mouseY: e.originalEvent.layerY,
 boardX: board.x,
 boardY: board.y,
 width: board.width,
 height: board.height 
 };
 if(Board.isInBounds(coord) && board.gameStatus == "turn"){
 var cell = board.getCell(coord);
 if(board.cell[cell.v][cell.h].player == ""){
 board.moves++;
 if(board.isXTurn){
 board.cell[cell.v][cell.h].drawX();
 board.isXTurn = !board.isXTurn;
 }else{
 board.cell[cell.v][cell.h].drawO();
 board.isXTurn = !board.isXTurn;
 }
 board.gameStatus = board.checkStatus(board);
 if(board.gameStatus == "turn"){
 board.drawStatusBar(board, "Player "+((board.isXTurn)?"X":"O")+" Turn!!");
 }else if(board.gameStatus == "win"){
 board.drawStatusBar(board, "Player "+((board.isXTurn)?"O":"X")+" Won!!");
 }else if(board.gameStatus == "tie"){
 board.drawStatusBar(board, "It is a Tie :D");
 } 
 }
 }
};

Cell.js

function Cell(data){
 data = (data === 'undefined')? {} : data;
 //$.extend(this, data);
 if(data){
 this.x = data.x;
 this.y = data.y;
 this.width = data.width;
 this.height = data.height;
 this.ctx = data.ctx;
 this.canvas = data.ctx;
 this.empty = true;
 this.player = "";
 }else{
 //should not construct the object
 }
}
Cell.prototype.clearCell = function(){
 this.ctx.clearRect(this.x+this.width/2+2, this.y+this.height/2+2, this.width-5, this.height-5);
 this.empty = true;
 this.player = "";
 //should clear the array that represents the board
};
Cell.prototype.drawX = function(){
 if(this.empty){
 var x = this.x+(2*this.width/3);
 var y = this.y+(2*this.height/3);
 //begin drawing the path for the x
 this.ctx.beginPath();
 //first line from the left
 this.ctx.moveTo(x, y);
 this.ctx.lineTo((x+(2*this.width/3)), (y+(2*this.height/3)));
 //second line from the right
 this.ctx.moveTo((x+(2*this.width/3)), y);
 this.ctx.lineTo(x, (y+(2*this.height/3)));
 //begin stroking the path then release it
 this.ctx.stroke();
 this.ctx.closePath();
 this.empty = false;
 this.player = "x";
 }
};
Cell.prototype.drawO = function(){ 
 if(this.empty){
 var x = this.x+(this.width);
 var y = this.y+(this.height);
 var radius = this.width/3;
 //begin setting the path to stroke
 this.ctx.beginPath();
 //set the path for the arc 
 this.ctx.arc(x, y, radius, 0, 180);
 //begin stroking the path then release it
 this.ctx.stroke();
 this.ctx.closePath();
 this.empty = false;
 this.player = "o";
 }
};
200_success
145k22 gold badges190 silver badges478 bronze badges
asked May 31, 2015 at 6:32
\$\endgroup\$
5
  • 2
    \$\begingroup\$ It would be fine as long as you are doing this based on separation of concern (en.wikipedia.org/wiki/Separation_of_concerns) and Single Responsibility principle (en.wikipedia.org/wiki/SOLID_%28object-oriented_design%29) \$\endgroup\$ Commented May 31, 2015 at 9:09
  • \$\begingroup\$ @Bob this check "if(board.cell[cell.v][cell.h].player == "")" makes sure this does not happen thanks for your effort \$\endgroup\$ Commented Jun 1, 2015 at 7:17
  • \$\begingroup\$ @Beatles1692 yes, I am familiar with those principles. i think i should read into them more thanks. \$\endgroup\$ Commented Jun 1, 2015 at 7:21
  • \$\begingroup\$ Sorry. I was too hasty. \$\endgroup\$ Commented Jun 1, 2015 at 15:07
  • \$\begingroup\$ @Bob No Problem :D \$\endgroup\$ Commented Jun 7, 2015 at 8:06

1 Answer 1

5
\$\begingroup\$

Dead Code

else{
 //undefined data object
 }

This isn't doing anything, so just delete it.

Enums

Right here, you are using a string literal to specify the player who needs to move:

this.player = "x";

Typically, you would use a player enum to specify the player.

Spacing

If you put spaces around your operators, your code is easier to read, debug (especially when finding order-of-operations bugs), and maintain:

this.ctx.moveTo((this.x+2*this.width/3), this.y);

Early Returns

Right here, you have some unnecessarily deep indentation:

if(Board.isInBounds(coord) && board.gameStatus == "turn"){
 var cell = board.getCell(coord);
 if(board.cell[cell.v][cell.h].player == ""){
 board.moves++;
 if(board.isXTurn){
 board.cell[cell.v][cell.h].drawX();
 board.isXTurn = !board.isXTurn;
 }else{
 board.cell[cell.v][cell.h].drawO();
 board.isXTurn = !board.isXTurn;
 }
 board.gameStatus = board.checkStatus(board);
 if(board.gameStatus == "turn"){
 board.drawStatusBar(board, "Player "+((board.isXTurn)?"X":"O")+" Turn!!");
 }else if(board.gameStatus == "win"){
 board.drawStatusBar(board, "Player "+((board.isXTurn)?"O":"X")+" Won!!");
 }else if(board.gameStatus == "tie"){
 board.drawStatusBar(board, "It is a Tie :D");
 } 
 }
}

If I wrote this, I would use early returns at the top to use less indentation:

if (!Board.isInBounds(coord) || board.gameStatus != "turn") {
 return;
}
var cell = board.getCell(coord);
if (board.cell[cell.v][cell.h].player != "") {
 return;
}
board.moves++;
if (board.isXTurn) {
 board.cell[cell.v][cell.h].drawX();
 board.isXTurn = !board.isXTurn;
} else {
 board.cell[cell.v][cell.h].drawO();
 board.isXTurn = !board.isXTurn;
}
board.gameStatus = board.checkStatus(board);
if (board.gameStatus == "turn") {
 board.drawStatusBar(board, "Player "+((board.isXTurn)?"X":"O")+" Turn!!");
} else if (board.gameStatus == "win"){
 board.drawStatusBar(board, "Player "+((board.isXTurn)?"O":"X")+" Won!!");
} else if (board.gameStatus == "tie"){
 board.drawStatusBar(board, "It is a Tie :D");
}
answered Jun 5, 2015 at 22:58
\$\endgroup\$
2
  • \$\begingroup\$ Okay first of all for the dead code issue. i was trying to make my code more general so when someone miss use it and does not send all needed parameters there should be an error state or something but i am still could not figure out how this should be done. I did not know that enums existed in js that is cool. Spacing i will take this into consideration thanks. This is first time i understand the importance of early returns and yes indeed the code looks more readable to me and much more easier. \$\endgroup\$ Commented Jun 7, 2015 at 8:04
  • \$\begingroup\$ @AbdulazizAlaa You could throw an exception in that else: stackoverflow.com/questions/464359/… \$\endgroup\$ Commented Jun 7, 2015 at 14:29

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.