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

Simon Says: is this too complex?

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

Problem

I took a class covering HTML, CSS, Javascript and for a final project, thinking of a previous community challenge, I created Simon Says.

Going through my code my professor deemed it overly complicated (or did he mean complex?), explicitly referring to my showSequence method in particular.

I'm just wondering what and how could I have simplified things?



function SimonButton(id, litColor, color) {
this.color = color;
this.id = id;
this.litColor = litColor;
this.highlight = function() {
document.getElementById(id).style.background = litColor;
setTimeout(function() {
document.getElementById(id).style.background = color;
}, 750);
}
}

var buttons = [
new SimonButton("alpha", "red", "darkred"),
new SimonButton("beta", "yellow", "darkorange"),
new SimonButton("gamma", "lightblue", "darkblue"),
new SimonButton("delta", "lightgreen", "darkgreen")
];

var sequence = [];
var round = 0;
var sequenceIterator = 0;
var userTurn = false;
playing = false;

window.onload = function() {
alert("Try to reproduce the sequence shown.\nClick start to begin!");

document.getElementById("newGame").onclick = function() {
document.getElementById("newGame").value = "New Game";
reset();
playing = true;
computerTurn();
}

for (var i = 0; i
body {
background-color: #272822;
}

input:focus {
outline: none;
}

input#newGame {
height: 3em;
width: 7em;
font-size: 15px;
font-family: Arial;
font-weight: bold;
border-radius: 6px;
text-decoration: none;
background-color: #A6E22E;
}
input#newGame:hover {
color: #fff;
}

div#counter {
position: relative;
font-weight: bold;
font-size: 30px;
font-family: Arial;
color: #fff;
left: 9em;
}

.simonButton {
height: 20em;
width: 20em;
margin: .5em;
}
.simonButton:hover {
box-shadow: 0 0 .5em #fff;
}

input#alpha {
background-color: darkred;
border-radius: 11em 2em 5em 2em;
}

input#alpha:active {
background-colo

Solution

You are missing a doctype for your HTML. The doctype tells the browser which version of HTML to parse it as, and the latest doctype is `, capitalization not required.

HTML
input elements of type button need a value to show the text on the button. As you do not want text on your buttons, you can create them with a single space, like:



The HTML validator at the W3C has no other complaints.

Your CSS also passes the W3C's validator.

When you declare your JS fields,
playing is not defined with var, but the others are:

playing = false;


Is this intentional, or no?

This statement can be confusing:
buttons[sequence[sequenceIterator++]].id`. I would recommend not incrementing an item in a statement that also retrieves a value.

Code Snippets

<input type='button' id='alpha' class='simonButton' value=' '>
playing = false;

Context

StackExchange Code Review Q#114181, answer score: 6

Revisions (0)

No revisions yet.