HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptModerate

Tic Tac Toe in plain JavaScript

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
toejavascripttictacplain

Problem

I have created Tic Tac Toe game using plain JavaScript. Can any javascript expert review it and let me know whether the way I have written JavaScript code that is right (preferable)?

I am unsure that I wrote the OnLoad function in a preferable way. I wanted wrap all my code in an immediate invoking function but that didn't work out well.

I just want to improve my JavaScript coding knowledge. I want to write JS code more professional way.

CodePen

HTML


Tic Tac Toe







Start a New Game








JS

var turn = 'X';
var score = {
'X': 0,
'O': 0
};
var gridValue = 0;

function fnLoad() {
var select = document.getElementById("grid");
for (i = 3; i = gridValue) {
var classes = targetElement.className.split(/\s+/);
for (i = 0; i

CSS

*{
-webkit-box-sizing: border-box;
-moz-box-sizing: border-box;
box-sizing: border-box;
}

select {
padding:3px;
margin: 0;
-webkit-border-radius:4px;
-moz-border-radius:4px;
border-radius:4px;
-webkit-box-shadow: 0 3px 0 #ccc, 0 -1px #fff inset;
-moz-box-shadow: 0 3px 0 #ccc, 0 -1px #fff inset;
box-shadow: 0 3px 0 #ccc, 0 -1px #fff inset;
background: #f8f8f8;
color:#888;
border:none;
outline:none;
display: inline-block;
-webkit-appearance:none;
-moz-appearance:none;
appearance:none;
cursor:pointer;
width:100px;
}

.minContainer {
padding: 20px;
padding-right: 30px;
position: absolute;
}

table {
border-collapse:collapse;
table-layout: fixed;
border-spacing: 0;
}
.td {
border: 2px solid #cccccc;
font-size:20px;
font-family:"Helvetica Neue", Helvetica, Arial, sans-serif;
color:#ccc;
line-height: 1.428571429;
width: 30px;
height: 32px;
min-width: 32px;
max-width: 32px;
min-hei

Solution

I'm not a JavaScript expert and you might want to wait for one. But here are a few things that stand out.

Variable declaration

You forgot to declare your loop variables. Instead of:

for (i = 3; i <= 100; i += 1) {
    // ... 
}


You should write like this:

for (var i = 3; i <= 100; ++i) {
    // ...
}


That is, declare the variable with var i. You should correct this in all your loops. And a tr variable in fnNewGame, should have been:

var tr = document.createElement('tr');


Also, it's customary to write ++i or i++ instead of i += 1.

Simplify isEven

There's no need to wrap a boolean expression in an if-else just to return a boolean:

function isEven(value) {
    if (value % 2 == 0)
        return true;
    else
        return false;
}


You can use the result of a boolean expression directly:

function isEven(value) {
    return value % 2 == 0;
}


Simplify notation

You don't have to, but you can write some expressions in a simpler way:

var score = {
    'X': 0,
    'O': 0
};


You can omit the quoting of the key names and write simpler as:

var score = {
    X: 0,
    O: 0
};


And you can access score.X and score.O instead of score['X'] and score['O'].

Unused variables

Remove unused variables, for example this:

var option = document.createElement('option');


Check yourself

To avoid common mistakes, simply copy-paste your code on http://jshint.com/ and fix the warnings it detects.

Code Snippets

for (i = 3; i <= 100; i += 1) {
    // ... 
}
for (var i = 3; i <= 100; ++i) {
    // ...
}
var tr = document.createElement('tr');
function isEven(value) {
    if (value % 2 == 0)
        return true;
    else
        return false;
}
function isEven(value) {
    return value % 2 == 0;
}

Context

StackExchange Code Review Q#60871, answer score: 11

Revisions (0)

No revisions yet.