patternjavascriptModerate
Tic Tac Toe in plain JavaScript
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
I just want to improve my JavaScript coding knowledge. I want to write JS code more professional way.
CodePen
HTML
JS
-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
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:
You should write like this:
That is, declare the variable with
Also, it's customary to write
Simplify
There's no need to wrap a boolean expression in an
You can use the result of a boolean expression directly:
Simplify notation
You don't have to, but you can write some expressions in a simpler way:
You can omit the quoting of the key names and write simpler as:
And you can access
Unused variables
Remove unused variables, for example this:
Check yourself
To avoid common mistakes, simply copy-paste your code on http://jshint.com/ and fix the warnings it detects.
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
isEvenThere'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.