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";
    }
};
\$\endgroup\$
  • 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\$ – Beatles1692 May 31 '15 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\$ – Abdulaziz Alaa Jun 1 '15 at 7:17
  • \$\begingroup\$ @Beatles1692 yes, I am familiar with those principles. i think i should read into them more thanks. \$\endgroup\$ – Abdulaziz Alaa Jun 1 '15 at 7:21
  • \$\begingroup\$ Sorry. I was too hasty. \$\endgroup\$ – Bob Jun 1 '15 at 15:07
  • \$\begingroup\$ @Bob No Problem :D \$\endgroup\$ – Abdulaziz Alaa Jun 7 '15 at 8:06
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");
}
\$\endgroup\$
  • \$\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\$ – Abdulaziz Alaa Jun 7 '15 at 8:04
  • \$\begingroup\$ @AbdulazizAlaa You could throw an exception in that else: stackoverflow.com/questions/464359/… \$\endgroup\$ – Hosch250 Jun 7 '15 at 14:29

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.