patternjavascriptMinor
Simon Says in Javascript (with Knockout) - Playable Code Snippet
Viewed 0 times
snippetjavascriptwithknockoutsimonplayablesayscode
Problem
I resolved to do the whole thing with just vanilla Javascript as a challenge, but man, Knockout's binding and state management is just too easy.
A tiny bit of callback hell to get all the flashing / guessing synchronized, but what I'd really like people to look into is the OO-style of this. I try to avoid anemic models wherever possible, and sometimes it makes my classes look ... bulky? So while it works, could it be improved from a code maintenance perspective if I (for example) broke out Pattern, Guess, etc into their own objects.
`var simonSquare = function(color) {
var self = this;
self.color = ko.observable(color);
self.isActive = ko.observable(false);
self.activeCSS = ko.computed(function() {
return self.isActive() ? "active" : "";
});
self.flash = function(callback) {
self.isActive(!self.isActive());
setTimeout(function() {self.isActive(!self.isActive());
if (callback) {
callback();
}}, 300);
};
};
var simon = function() {
var self = this;
self.Squares = ko.observableArray([
new simonSquare('red'),
new simonSquare('blue'),
new simonSquare('green'),
new simonSquare('yellow')
]);
self.Pattern = [];
self.playPattern = function(index) {
var index = index || 0;
self.Pattern[index].flash(function() {
if (self.Pattern[index + 1]) {
setTimeout(function() {
self.playPattern(index + 1);
}, 200) ;
}
else {
self.canGuess = true;
}
});
};
self.highScore = ko.observable(0);
//Stack Snippets are sandboxed, no local storage
//self.highScore = ko.observable(localStorage.getItem("SimonSaysHighScore"));
self.canGuess = false;
self.gameOver = ko.observable(false);
self.currentPositionBeingGuessed = 0;
self.guess = function(guessedSquare) {
if (!self.canGuess) return;
guessedSquare.flash(function() {
if (self.Pattern[self.currentPositionBeingGuessed] == guessedSquare) {
if (self.currentPositionBeingGuessed == (self.Pattern.length -1 )) { // completed pattern
if (self.P
A tiny bit of callback hell to get all the flashing / guessing synchronized, but what I'd really like people to look into is the OO-style of this. I try to avoid anemic models wherever possible, and sometimes it makes my classes look ... bulky? So while it works, could it be improved from a code maintenance perspective if I (for example) broke out Pattern, Guess, etc into their own objects.
`var simonSquare = function(color) {
var self = this;
self.color = ko.observable(color);
self.isActive = ko.observable(false);
self.activeCSS = ko.computed(function() {
return self.isActive() ? "active" : "";
});
self.flash = function(callback) {
self.isActive(!self.isActive());
setTimeout(function() {self.isActive(!self.isActive());
if (callback) {
callback();
}}, 300);
};
};
var simon = function() {
var self = this;
self.Squares = ko.observableArray([
new simonSquare('red'),
new simonSquare('blue'),
new simonSquare('green'),
new simonSquare('yellow')
]);
self.Pattern = [];
self.playPattern = function(index) {
var index = index || 0;
self.Pattern[index].flash(function() {
if (self.Pattern[index + 1]) {
setTimeout(function() {
self.playPattern(index + 1);
}, 200) ;
}
else {
self.canGuess = true;
}
});
};
self.highScore = ko.observable(0);
//Stack Snippets are sandboxed, no local storage
//self.highScore = ko.observable(localStorage.getItem("SimonSaysHighScore"));
self.canGuess = false;
self.gameOver = ko.observable(false);
self.currentPositionBeingGuessed = 0;
self.guess = function(guessedSquare) {
if (!self.canGuess) return;
guessedSquare.flash(function() {
if (self.Pattern[self.currentPositionBeingGuessed] == guessedSquare) {
if (self.currentPositionBeingGuessed == (self.Pattern.length -1 )) { // completed pattern
if (self.P
Solution
So one thing I've learned from doing my own code review is how much I just blindly use Knockout observables and observableArrays even when vanilla ones will do.
The basic Knockout rule of thumb is: if you aren't using it to modify the HTML or manage pub/sub events, you can probably just make it vanilla JS instead.
For example:
-
Rather than use a ko.computed for the CSS based on a boolean, just set the CSS observable directly in flash().
Now the only Knockout things remaining that could be converted to vanilla JS are:
-
The flash method which uses the css binding to turn active on and off each square. I'll need to modify it to take in an element ID from a clicked square.
-
The foreach binding to render the simonSquares' HTML using a single template instead of manually creating them or doing some hokey document.write() call.
Although we could convert the buttons to static DIVs and then have them flash and be guessed using their element IDs, this is in fact where Knockout's elegance really shines and what it's really made for. So I'm going to leave these alone.
The basic Knockout rule of thumb is: if you aren't using it to modify the HTML or manage pub/sub events, you can probably just make it vanilla JS instead.
For example:
- We're not dynamically adding and removing Simon Squares, so Squares becomes a vanilla array
- Nor are we changing the colors on the fly, so color in simonSquare becomes a vanilla property
- Nor are we displaying the Pattern on the screen or otherwise giving the users access to it, so again, another vanilla array
- In this particular version, the gameOver and highScore observables are only used to manipulate two info messages, so I replaced their observable logic with some boring old document.getElementById manipulators.
-
Rather than use a ko.computed for the CSS based on a boolean, just set the CSS observable directly in flash().
var simonSquare = function(color) {
var self = this;
self.color = color;
self.activeCSS = ko.observable("");
self.flash = function(callback) {
self.activeCSS("active");
setTimeout(function() {self.activeCSS("");
if (callback) {
callback();
}}, 300);
};
};
var simon = function() {
var self = this;
self.Squares = [
new simonSquare('red'),
new simonSquare('blue'),
new simonSquare('green'),
new simonSquare('yellow')
];
self.Pattern = [];
self.playPattern = function(index) {
var index = index || 0;
self.Pattern[index].flash(function() {
if (self.Pattern[index + 1]) {
setTimeout(function() {
self.playPattern(index + 1);
}, 200) ;
}
else {
self.canGuess = true;
}
});
};
self.highScore = 0;
//Stack Snippets are sandboxed, no local storage
//self.highScore = ko.observable(localStorage.getItem("SimonSaysHighScore"));
self.canGuess = false;
self.currentPositionBeingGuessed = 0;
self.guess = function(guessedSquare) {
if (!self.canGuess) return;
guessedSquare.flash(function() {
if (self.Pattern[self.currentPositionBeingGuessed] == guessedSquare) {
if (self.currentPositionBeingGuessed == (self.Pattern.length -1 )) { // completed pattern
if (self.Pattern.length > self.highScore) {
self.highScore = self.Pattern.length;
document.getElementById('highScore').innerHTML = self.highScore;
}
self.newRound();
}
else {
self.currentPositionBeingGuessed++;
}
}
else {
document.getElementById('gameOverMessage').style.display = 'block';
/* Stack Snippets are sandboxed, no local storage
if(typeof(Storage) !== "undefined") {
localStorage.setItem("SimonSaysHighScore", self.Pattern.length -1);
} */
self.canGuess = false;
}
});
}
self.newRound = function() {
self.canGuess = false;
self.currentPositionBeingGuessed = 0;
self.Pattern.push(self.Squares[Math.floor(Math.random()*self.Squares.length)]);
setTimeout(self.playPattern, 500);
}
self.newGame = function() {
document.getElementById('gameOverMessage').style.display = 'none';
self.Pattern = [];
self.newRound();
};
};Now the only Knockout things remaining that could be converted to vanilla JS are:
-
The flash method which uses the css binding to turn active on and off each square. I'll need to modify it to take in an element ID from a clicked square.
-
The foreach binding to render the simonSquares' HTML using a single template instead of manually creating them or doing some hokey document.write() call.
- Similary, The guess method which uses Knockout's awesome data binding to receive the simonSquare viewModel sitting underneath the button when it's clicked.
Although we could convert the buttons to static DIVs and then have them flash and be guessed using their element IDs, this is in fact where Knockout's elegance really shines and what it's really made for. So I'm going to leave these alone.
Code Snippets
var simonSquare = function(color) {
var self = this;
self.color = color;
self.activeCSS = ko.observable("");
self.flash = function(callback) {
self.activeCSS("active");
setTimeout(function() {self.activeCSS("");
if (callback) {
callback();
}}, 300);
};
};
var simon = function() {
var self = this;
self.Squares = [
new simonSquare('red'),
new simonSquare('blue'),
new simonSquare('green'),
new simonSquare('yellow')
];
self.Pattern = [];
self.playPattern = function(index) {
var index = index || 0;
self.Pattern[index].flash(function() {
if (self.Pattern[index + 1]) {
setTimeout(function() {
self.playPattern(index + 1);
}, 200) ;
}
else {
self.canGuess = true;
}
});
};
self.highScore = 0;
//Stack Snippets are sandboxed, no local storage
//self.highScore = ko.observable(localStorage.getItem("SimonSaysHighScore"));
self.canGuess = false;
self.currentPositionBeingGuessed = 0;
self.guess = function(guessedSquare) {
if (!self.canGuess) return;
guessedSquare.flash(function() {
if (self.Pattern[self.currentPositionBeingGuessed] == guessedSquare) {
if (self.currentPositionBeingGuessed == (self.Pattern.length -1 )) { // completed pattern
if (self.Pattern.length > self.highScore) {
self.highScore = self.Pattern.length;
document.getElementById('highScore').innerHTML = self.highScore;
}
self.newRound();
}
else {
self.currentPositionBeingGuessed++;
}
}
else {
document.getElementById('gameOverMessage').style.display = 'block';
/* Stack Snippets are sandboxed, no local storage
if(typeof(Storage) !== "undefined") {
localStorage.setItem("SimonSaysHighScore", self.Pattern.length -1);
} */
self.canGuess = false;
}
});
}
self.newRound = function() {
self.canGuess = false;
self.currentPositionBeingGuessed = 0;
self.Pattern.push(self.Squares[Math.floor(Math.random()*self.Squares.length)]);
setTimeout(self.playPattern, 500);
}
self.newGame = function() {
document.getElementById('gameOverMessage').style.display = 'none';
self.Pattern = [];
self.newRound();
};
};Context
StackExchange Code Review Q#71783, answer score: 5
Revisions (0)
No revisions yet.