patternjavascriptMinor
Simon Says: is this too complex?
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
I'm just wondering what and how could I have simplified things?
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
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.