11
\$\begingroup\$

As an exercise, I decided to create a simple Tic-Tac-Toe game. It is Ruby on Rails based, but as for now I'm not using the server side for anything (I intend to build up on it in the future, though).

As I'm rather new with JavaScript, HTML5 and CSS, I'd like some feedback regarding what did I do wrong or what could be done better.

index.html.erb

<!DOCTYPE html>

<div id="container">
  <div class="row">
    <canvas class="field" id="f11" width="150" height="150" token="None"></canvas>
    <canvas class="field" id="f12" width="150" height="150" token="None"></canvas>
    <canvas class="field" id="f13" width="150" height="150" token="None"></canvas>
  </div>
  <div class="row">
    <canvas class="field" id="f21" width="150" height="150" token="None"></canvas>
    <canvas class="field" id="f22" width="150" height="150" token="None"></canvas>
    <canvas class="field" id="f23" width="150" height="150" token="None"></canvas>
  </div>
  <div class="row">
    <canvas class="field" id="f31" width="150" height="150" token="None"></canvas>
    <canvas class="field" id="f32" width="150" height="150" token="None"></canvas>
    <canvas class="field" id="f33" width="150" height="150" token="None"></canvas>
  </div>
  <div class="row">
    <canvas id="reset" width="150" height="50"></canvas>
  </div>
</div>

tic_tac_toe.js

var tokenAttributeName = 'token';
var idAttributeName = 'id';

var circleTokenName = 'Circle';
var crossTokenName = 'Cross';
var noTokenName = 'None';

var circleColor = 'green';
var crossColor = 'red';
var fontColor = 'black';
var font = "bold 30px Helvetica";

var resetText = "Reset";

var hoverOnOpacity = 1.0;
var hoverOffOpacity = 0.75;
var highlightTime = 100;

var turn = circleTokenName;
var won = false;

$(document).ready(function() {
  drawResetCanvas();

  $('canvas#reset').click(function(e) {
    location.reload();
  });

  $('canvas').hover(
    function(e) {
      var field = $(this);

      field.animate({ opacity: hoverOnOpacity }, highlightTime);
    },
    function(e) {
      var field = $(this);

      field.animate({ opacity: hoverOffOpacity }, highlightTime);
  });

  $('canvas.field').click(function(e) {
    var field = $(this);
    var fieldId = field.attr(idAttributeName);

    if (field.attr(tokenAttributeName) != noTokenName || won) {
      return;
    }

    if (turn == circleTokenName) {
      drawCircle(fieldId);
      field.attr(tokenAttributeName, circleTokenName);
    }
    else {
      drawCross(fieldId);
      field.attr(tokenAttributeName, crossTokenName);
    }

    //field.animate({opacity: 1.0}, 0, function() { field.animate({opacity:0.75},2000)});

    if (checkWin(turn)) {
      alert(turn + ' won!');
      won = true;
    }
    else if (checkDraw()) {
      alert('Draw!');
    };

    turn = turn == circleTokenName ? crossTokenName : circleTokenName;
  });
});

function checkWin(figure) {
  return (
    checkRows(figure) ||
    checkCols(figure) ||
    checkDiagonals(figure)
  );
};

function checkRows(figure) {
  return (
    checkRow(1, figure) ||
    checkRow(2, figure) ||
    checkRow(3, figure)
  );
};

function checkRow(rowId, figure) {
  return (
    checkField(rowId, 1, figure) &&
    checkField(rowId, 2, figure) &&
    checkField(rowId, 3, figure)
  );
};

function checkCols(figure) {
  return (
    checkCol(1, figure) ||
    checkCol(2, figure) ||
    checkCol(3, figure)
  );
};

function checkCol(colId, figure) {
  return (
    checkField(1, colId, figure) &&
    checkField(2, colId, figure) &&
    checkField(3, colId, figure)
  );
};

function checkDiagonals(figure) {
  return (
    checkField(1, 1, figure) &&
    checkField(2, 2, figure) &&
    checkField(3, 3, figure)
  ) ||
  (
    checkField(1, 3, figure) &&
    checkField(2, 2, figure) &&
    checkField(3, 1, figure)
  );
};

function checkDraw() {
  return !(
    checkField(1, 1, noTokenName) ||
    checkField(1, 2, noTokenName) ||
    checkField(1, 3, noTokenName) ||
    checkField(2, 1, noTokenName) ||
    checkField(2, 2, noTokenName) ||
    checkField(2, 3, noTokenName) ||
    checkField(3, 1, noTokenName) ||
    checkField(3, 2, noTokenName) ||
    checkField(3, 3, noTokenName)
  );
};

function checkField(rowId, colId, figure) {
  return $('canvas#f' + rowId + colId).attr(tokenAttributeName) == figure
}

function drawCircle(fieldId) {
  var canvas =  document.getElementById(fieldId);
  var context = canvas.getContext('2d');
  var centerX = canvas.width / 2;
  var centerY = canvas.height / 2;
  var innerRadius = 0.5 * canvas.width / 2;
  var outerRadius = 0.75 * canvas.width / 2;

  context.beginPath();
  context.arc(centerX, centerY, outerRadius, 0, 2 * Math.PI, false);
  context.arc(centerX, centerY, innerRadius, 0, 2 * Math.PI, true);
  context.fillStyle = circleColor;
  context.fill();
};

function drawCross(fieldId) {
  var canvas =  document.getElementById(fieldId);
  var context = canvas.getContext('2d');
  var centerX = canvas.width / 2;
  var centerY = canvas.height / 2;

  var innerMostPointsOffset = 0.20 * canvas.width / 2;
  var innerEndingPointsXOffset = 0.4 * canvas.width / 2;
  var innerEndingPointsYOffset = 0.75 * canvas.width / 2;
  var outerEndingPointsXOffset = 0.75 * canvas.width / 2;
  var outerEndingPointsYOffset = 0.75 * canvas.width / 2;

  context.fillStyle = crossColor;
  context.beginPath();
  context.moveTo(centerX - innerMostPointsOffset, centerY);
  context.lineTo(centerX - outerEndingPointsXOffset, centerY - outerEndingPointsYOffset);
  context.lineTo(centerX - innerEndingPointsXOffset, centerY - innerEndingPointsYOffset);
  context.lineTo(centerX, centerY - innerMostPointsOffset);
  context.lineTo(centerX + innerEndingPointsXOffset, centerY - innerEndingPointsYOffset);
  context.lineTo(centerX + outerEndingPointsXOffset, centerY - outerEndingPointsYOffset);
  context.lineTo(centerX + innerMostPointsOffset, centerY);
  context.lineTo(centerX + outerEndingPointsXOffset, centerY + outerEndingPointsYOffset);
  context.lineTo(centerX + innerEndingPointsXOffset, centerY + innerEndingPointsYOffset);
  context.lineTo(centerX, centerY + innerMostPointsOffset);
  context.lineTo(centerX - innerEndingPointsXOffset, centerY + innerEndingPointsYOffset);
  context.lineTo(centerX - outerEndingPointsXOffset, centerY + outerEndingPointsYOffset);
  context.closePath();
  context.fill();
};

function drawResetCanvas() {
  var canvas = document.getElementById("reset");
  var context = canvas.getContext("2d");
  context.fillStyle = fontColor;
  context.font = font;
  context.fillText(resetText, 35, 35);
};

tic_tac_toe.css

canvas {
  background-color: #DDDDDD;
  width: 150px;
  border: 1px solid white;
  float: left;
  opacity: 0.75;
}

canvas.field {
  height: 150px;
}

div.row {
  width: 456px;
  margin: 0 auto;
}

div#container {
  width: 100%;
}

canvas#reset {
  height: 50px;
  margin-top: 30px;
  margin-left: 152px;
}
\$\endgroup\$
1

2 Answers 2

Reset to default
7
\$\begingroup\$

First, a (proper) HTML document looks like this:

<!DOCTYPE html>
<html>
    <head>
        <title>Tic tac toe</title>
    </head>
    <body>
        <!-- Stuff -->
    </body>
</html>

Html, head, body, and title are strongly recommended, tho not required.

You seem to be using <canvas>s a lot… why? Can't you just make them images?

<div class="row">
    <img class="field" />
    <img class="field" />
    <img class="field" />
</div>
<div class="row">
    <img class="field" />
    <img class="field" />
    <img class="field" />
</div>
<div class="row">
    <img class="field" />
    <img class="field" />
    <img class="field" />
</div>

Canvases are ugly and pixely, images are beautiful and potentially vector (or at least easier to make high-res (important for retina devices)). The only downside is you have to create the images… but I just did it for you (tweak to your liking):

<svg xmlns="http://www.w3.org/2000/svg" width="50" height="50" fill="#f00"> <!-- An X -->
    <polygon points="0,4 4,0 50,46 46,50" />
    <polygon points="50,4 4,50 0,46 46,0" />
</svg>
<svg xmlns="http://www.w3.org/2000/svg" width="50" height="50" stroke="#090" stroke-width="5" fill="none"> <!-- An O -->
    <circle cx="25" cy="25" r="22" />
</svg>

Don't make a bajillion ids. Instead of $('canvas#f' + rowId + colId):

document.getElementById('container').children[rowId].children[colId]

Yeah, I hate jQuery. Especially the animations. CSS transitions are supported wide enough that you should probably use that instead. It's smoother - the DOM isn't meant to be messed around with every 20ms (as is the case with jQuery animations).

So, pretty good job for recent unzoomed desktop browsers with users who can't notice jQuery's slowness. Now just make it vector and vanilla and not have a bunch of… things in the DOM (ids, "tokens" (invalid attribute)) which can be put in arrays.

\$\endgroup\$
3
\$\begingroup\$

I will only focus on your method to determine wins / draws, if you need so much code then you are probably doing something wrong ;)

Given that tic tac toe is 3 by 3, I find that simply hard-coding is unbeatable in expressing what you want the logic to check.

Furthermore, for determining a draw, if you are going to use jQuery, you might as well ask through jQuery whether any element has the attribute value noTokenName.

So I would counter suggest something like this:

function checkWin(figure) {
  var winningTriplets = [
    [ [1,1] , [1,2] , [1,3] ], //Column 1
    [ [2,1] , [2,2] , [2,3] ], //Column 2
    [ [3,1] , [3,2] , [3,3] ], //Column 3
    [ [1,1] , [2,1] , [3,1] ], //Row 1
    [ [1,2] , [2,2] , [3,2] ], //Row 1
    [ [1,3] , [2,3] , [3,3] ], //Row 1
    [ [1,1] , [2,2] , [3,3] ], //Diagonal 1
    [ [1,3] , [2,2] , [3,1] ]  //Diagonal 1
  ];

  for( var i = 0 ; i < winningTriplets.length ; i++ ){
    if( checkTriplet( winningTriplets[i] , figure ) ){
      return true;
    }
  }
};

/* Check 3 coordinates for a given triplet
   a triplet is an array with 3 entries ( arrays ) with 2 entries 
   index 0 is col, index 1 row */
function checkTriplet( triplet , figure) {
  var X = 0, Y = 1;
  return (
    checkField( triplet[0][X], triplet[0][Y] , figure) ||
    checkField( triplet[1][X], triplet[1][Y] , figure) ||
    checkField( triplet[2][X], triplet[2][Y] , figure);    
  );
};

function checkDraw() {
  var queryString = "[" + tokenAttributeName + "='" + noTokenName + "']";
  return !$( queryString ).length
};

function checkField(rowId, colId, figure) {
  return $('canvas#f' + rowId + colId).attr(tokenAttributeName) == figure
}

The biggest problem I see, is that you are using the HTML elements to contain the state of your game. HTML elements were not meant for that. You should have a JS object with the game state, and then using that object you can draw the game.

However, since this is a small game, I guess you can get away with your approach.

\$\endgroup\$

Your Answer

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

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