JavaScript Tic-Tac-Toe review: best practices, correctness, and any other improvements/suggestions

Question

I just finished a little game of Tic-Tac-Toe for a class I'm in, and I think it looks alright, but I'm pretty fresh, so I would like to know how I might be able to improve it, given the situation I'm in. I'd like to host it on my website and I'd like it to be safe and expandable.

Please let me know where I'm off (or gone wrong) in terms of best practices and if you feel like being nice, what looks OK. I know I've got three global variables, but the program is small, so I let them stay there. If you can think of a way to easily eliminate those variables without drastically changing the rest of the code, I'd be interested. Again, I'd like this to become something else (a more complicated game of Tic-Tac-Toe) so if you can think of ways of making the code more usable at a future time, I'd also be interested.

JS:

    <script type="text/javascript">
        // Global variables
        var firstGame = true;
        var turnNum;
        var squareTaken = new Array();

        //functions
        function newGame(firstGame){ //starts a new game by
            turnNum = 0;             //setting the turn number to 0
            for (var i=0; i<9; i++){ //populating squareTaken array
                squareTaken[i] = ""; //with "", as in no X or O
            }
            if (firstGame !== true){  //if this is not the first game
                var canvasID;
                var c;
                var cxt;
                for (var i= 0; i<9; i++){ //loop through the canvas contexts and erase their current drawings.
                    canvasID = "canvas" + i;
                    c = document.getElementById(canvasID);
                    cxt = c.getContext("2d");
                    cxt.clearRect(0,0,50,50); //Clear the rectangle starting at 0,0 and going to the width and height of each.
                }
            }
        }

        function canvasClicked(canvasNum){ //handles canvas clicks
            var canvasID;
            var c;
            var cxt;
            canvasID = "canvas" + canvasNum;
            c = document.getElementById(canvasID);
            cxt = c.getContext("2d");

            if (squareTaken[canvasNum] == ""){
                if (turnNum % 2 == 0){
                    cxt.beginPath();
                    cxt.moveTo(10,10);
                    cxt.lineTo(40,40);
                    cxt.moveTo(10,40);
                    cxt.lineTo(40,10);
                    cxt.stroke();
                    cxt.closePath();
                    squareTaken[canvasNum] = "X";
                    turnNum++;
                }
                else {
                    cxt.beginPath();
                    cxt.arc(25,25,15,0,2*Math.PI)
                    cxt.stroke();
                    cxt.closePath();
                    squareTaken[canvasNum] = "O";
                    turnNum++;
                }
            }
            if (turnNum > 4){
                checkWin(squareTaken, canvasNum);
            }
        }

        function checkWin(squareTaken, canvasNum){ //returns true if game over
            if ( //If any of the following combinations are true, someone has won.  last && checks for non-occupied squares.
            (squareTaken[0]===squareTaken[1] && squareTaken[1]===squareTaken[2] && squareTaken[0] != "") || 
            (squareTaken[3]===squareTaken[4] && squareTaken[4]===squareTaken[5] && squareTaken[3] != "") ||
            (squareTaken[6]===squareTaken[7] && squareTaken[7]===squareTaken[8] && squareTaken[6] != "") ||
            (squareTaken[0]===squareTaken[3] && squareTaken[3]===squareTaken[6] && squareTaken[0] != "") ||
            (squareTaken[1]===squareTaken[4] && squareTaken[4]===squareTaken[7] && squareTaken[1] != "") ||
            (squareTaken[2]===squareTaken[5] && squareTaken[5]===squareTaken[8] && squareTaken[2] != "") ||
            (squareTaken[0]===squareTaken[4] && squareTaken[4]===squareTaken[8] && squareTaken[0] != "") ||
            (squareTaken[2]===squareTaken[4] && squareTaken[4]===squareTaken[6] && squareTaken[2] != "")
            )
            {
                //Declare winner
                alert(squareTaken[canvasNum] + " wins!\nStarting a new game!");
                if (firstGame === true){
                    firstGame = false;
                }
                newGame(firstGame);
            }
            else if ( //Check for tie
            squareTaken[0]!="" && squareTaken[1]!="" && squareTaken[2]!="" && 
            squareTaken[3]!="" && squareTaken[4]!="" && squareTaken[5]!="" && 
            squareTaken[6]!="" && squareTaken[7]!="" && squareTaken[8]!=""
            )
            {
                //Declare tie
                alert("It's a tie! \n Starting new game!");
                if (firstGame === true){
                    firstGame = false;
                }
                newGame(firstGame);
            }
        }


        //Start the game
        newGame(firstGame);

    </script>

Related HTML:

    <canvas id="canvas0" width="50" height="50" onClick="canvasClicked(0)"></canvas>
    <canvas id="canvas1" width="50" height="50" onClick="canvasClicked(1)"></canvas>
    <canvas id="canvas2" width="50" height="50" onClick="canvasClicked(2)"></canvas>
    <br />
    <canvas id="canvas3" width="50" height="50" onClick="canvasClicked(3)"></canvas>
    <canvas id="canvas4" width="50" height="50" onClick="canvasClicked(4)"></canvas>
    <canvas id="canvas5" width="50" height="50" onClick="canvasClicked(5)"></canvas>
    <br />
    <canvas id="canvas6" width="50" height="50" onClick="canvasClicked(6)"></canvas>
    <canvas id="canvas7" width="50" height="50" onClick="canvasClicked(7)"></canvas>
    <canvas id="canvas8" width="50" height="50" onClick="canvasClicked(8)"></canvas>

Answer 1

As @Robz says I would have a single canvas element and detect the mouse position. This will make it easier to draw a grid and to blank the canvas when restarting the game. It's also easier to track event handling in the javascript (e.g. canvas.onclick = canvasClicked) rather than in the HTML.

For any more complicated programme you would probably want to separate the drawing from the game logic by having a set of functions, or better, a module object, that takes care of drawing.

I would tend to have a for loop to check the different possible ways of winning or drawing rather than one very long if condition, although your way of doing it is at least simple and transparent.

You start a new game from within the checkWin function, which in turn is called from canvasClicked. This is okay but could get confusing if you put code after the call to newGame, which will then be carried out after the new game has been set up.

Finally

if (firstGame === true){
                    firstGame = false;
                }

can be simplified to

firstGame = false

Here is a version illustrating the points about the drawing and event handling, replacing the lengthy if conditions with loops, and showing how you could have separate canvas and game modules.

Answer 2

To start with, you can eliminate all your global variables by putting your code in an anonymous function, and then immediate invoke that function:

<script type="text/javascript">
(function () {
    // all of your code
}());
</script>

Note that your functions are global variables too, and pollute the global namespace just as much. To illustrate, this:

function foo () { ... };

Is practically the same thing as this:

var foo = function () { ... };

Also, you might know this, but I'll mention it anyway: unlike C++ or Java, javascript is function-scoped, so any variables you declare in conditionals or loops are accessible in the enclosing function. So, this:

var canvasID;
var c;
var cxt;
var i;

if (firstGame !== true){  //if this is not the first game
    for (i= 0; i<9; i++){ //loop through the canvas contexts and erase their current drawings.
        canvasID = "canvas" + i;
        c = document.getElementById(canvasID);
        cxt = c.getContext("2d");
        cxt.clearRect(0,0,50,50); //Clear the rectangle starting at 0,0 and going to the width and height of each.
    }
}

Is equivalent, in terms of scope, to your code:

if (firstGame !== true){  //if this is not the first game
    var canvasID;
    var c;
    var cxt;
    for (var i= 0; i<9; i++){ //loop through the canvas contexts and erase their current drawings.
        canvasID = "canvas" + i;
        c = document.getElementById(canvasID);
        cxt = c.getContext("2d");
        cxt.clearRect(0,0,50,50); //Clear the rectangle starting at 0,0 and going to the width and height of each.
    }
}

(After the if statement the variable canvaseID, for example, still exists can still be used--in both your code and mine above.) Not understanding this can cause bewildering bugs. When I was first getting used to function-scoped variables I would declare all variables in one var statement at the top of the function.

On using canvas: instead of have 9 canvas elements, I would use one canvas element, and divide it up into 9 squares in code. The way you've done it is fine, though.

If you're really interested in best practices with javascript, you may be interested in using jslint.