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

Nonogram game in JavaScript

Submitted by: @import:stackexchange-codereview··
0
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.

  • 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

-
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.