patternjavascriptMinor
Nonogram game in JavaScript
Viewed 0 times
javascriptgamenonogram
Problem
I am writing a Nonogram game in JavaScript.
I would like to know your opinions on the code and suggestions on how to improve it.
I'm using a MVC approach.
Here is the source code (I am going to release it under the GNU GPL license):
utils.js
nonograms.js
``
| | | | (_) | (_| | __/ |
|_| |_|\___/ \__,_|\___|_|
*/
koala.nonograms.Model = function (opts) {
if (!opts) opts = {};
this._opts = opts;
this.width = opts.width = (opts.width ? Number(opts.width) : 10);
this.height = opts.height = (opts.height ? Number(opts.height) : 10);
// Events fired by the model
this.events = {};
// The ussr guessed the content of a cell
this.events.guessChanged = new koala.utils.Event(this);
// The nonogr
I would like to know your opinions on the code and suggestions on how to improve it.
I'm using a MVC approach.
- the Model has two matrices (represented as linear arrays): one with the actual Nonogram cells and the other with the guesses of the player;
- cells are just strings: Nonogram cells can be either "filled" or "empty"; player guesses can be either "unknown", "filled" or "empty"
- by sequence I mean a sequence of consecutive filled cells
- by definition I mean the series of sequences a row/column has
Here is the source code (I am going to release it under the GNU GPL license):
utils.js
if (!window.koala) {
window.koala = {};
}
koala.utils = {
randomIntegerInRange: function (min, max) {
return min + Math.floor(Math.random() * (max - min + 1));
}
};
koala.utils.Event = function (sender) {
this._sender = sender;
this._listeners = [];
};
koala.utils.Event.prototype = {
attach: function (listener) {
this._listeners.push(listener);
},
notify: function (args) {
for (var index = 0, nlisteners = this._listeners.length; index < nlisteners; index++) {
this._listeners[index](this._sender, args);
}
}
};nonograms.js
``
if (!window.koala) {
window.koala = {};
}
koala.nonograms = {};
/*
The game uses the MVC pattern.
*/
/*
__ __ _ _
| \/ | | | | |
| \ / | ___ __| | ___| |
| |\/| |/ _ \ / _ |/ _ \ || | | | (_) | (_| | __/ |
|_| |_|\___/ \__,_|\___|_|
*/
koala.nonograms.Model = function (opts) {
if (!opts) opts = {};
this._opts = opts;
this.width = opts.width = (opts.width ? Number(opts.width) : 10);
this.height = opts.height = (opts.height ? Number(opts.height) : 10);
// Events fired by the model
this.events = {};
// The ussr guessed the content of a cell
this.events.guessChanged = new koala.utils.Event(this);
// The nonogr
Solution
In general, it looks good to me.
My 2 cents:
-
I am not sure why you have event related things in your model
-
Taking that into account, you could try something like the above.
My 2 cents:
-
I am not sure why you have event related things in your model
-
iterateOverDraggedCells looked like it could use some refactoring.- Whenever you start with x1,y1,x2,y2, I think it is better to have a point object with x and y.
- You can use Math.min() and Math.max() to find which value is the lowest/highest
- It looks like you went too far with DRY in that last for() loop
iterateOverDraggedCells: function ( fn )
{
var p1 = { x : this._x1 , y : this._y1 },
p2 = { x : this._x2 , y : this._y2 },
from, to;
if( Math.abs( p1.x - p2.x ) > Math.abs( p1.y - p2.y ) )
{
from = { x : Math.min( p1.x ,p2.x ) , y : p1.y };
to = { x : Math.max( p1.x ,p2.x ) };
for( var x = from.x, y = from.y ; x <= to.x , x++ )
fn(x, y, this._guess);
}
else
{
from = { y : Math.min( p1.y , p2.y ) , x : p1.x };
to = { y : Math.max( p1.y , p2.y ) };
for( var x = from.x, y = from.y ; y <= to.y , y++ )
fn(x, y, this._guess);
}
}Taking that into account, you could try something like the above.
Code Snippets
iterateOverDraggedCells: function ( fn )
{
var p1 = { x : this._x1 , y : this._y1 },
p2 = { x : this._x2 , y : this._y2 },
from, to;
if( Math.abs( p1.x - p2.x ) > Math.abs( p1.y - p2.y ) )
{
from = { x : Math.min( p1.x ,p2.x ) , y : p1.y };
to = { x : Math.max( p1.x ,p2.x ) };
for( var x = from.x, y = from.y ; x <= to.x , x++ )
fn(x, y, this._guess);
}
else
{
from = { y : Math.min( p1.y , p2.y ) , x : p1.x };
to = { y : Math.max( p1.y , p2.y ) };
for( var x = from.x, y = from.y ; y <= to.y , y++ )
fn(x, y, this._guess);
}
}Context
StackExchange Code Review Q#27689, answer score: 5
Revisions (0)
No revisions yet.