current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 4 down vote favorite

I'm writing a simple JavaScript game, which uses a 2-dimensional array as the main data structure. I wrote this function-class-thing that allows me to give any two integers, positive or negative. This class should "wrap" those integers back onto the game grid.

For example:

var g = new Grid(10, 10)
g.set(15, -2, "hello")
g.get(5, 8); //should be "hello"

Anyway, I'm striving to improve my JavaScript, so I'm wondering if there are any improvements (performance, "javascriptness", etc.) that I might be able to make.

var Grid = (function(w, h){
    this._width = w;
    this._height = h;
    this._grid = new Array((this._width * this._height) | 0);
    this.get = function(x, y){
            var coords = _normalize(x, y);
            return this._grid[(coords.y * coords.x) + coords.x];
    }
    this.set = function(x, y, item){
            var coords = _normalize(x, y);
            this._grid[(coords.y * coords.x) + coords.x] = item;
    }
    this._normalize = function(x, y){
            if(x < 0) x = (this._width + x) % this._width;
            else if(x >= this._width) x = x % this._width ;
            if(y < 0) y = (this._height + y) % this._height;
            else if(y >= this._height) y = y % this._height;
            return {'x': x, 'y': y};
    }
});
share|improve this question
    
What should happen if x = -11 and width = 10? –  Just_Mad Oct 10 '13 at 8:29

1 Answer 1

up vote 3 down vote

I can't say, that my variant of normalize function is easier to understand, but at least it is linear and it works correct (as i think) with x, that is less than -width 

this._normalize = function(x, y) {
    x = x % this._width;
    y = y % this._height;

    x = (this._width + x) % this._width;
    y = (this._height + y) % this._height;

    return {'x': x, 'y': y};
}

Also, you missed this statement in get and set functions (looks like a typo):

this.get = function(x, y){
    var coords = this._normalize(x, y);
    return this._grid[(coords.y * coords.x) + coords.x];
}
this.set = function(x, y, item){
    var coords = this._normalize(x, y);
    this._grid[(coords.y * coords.x) + coords.x] = item;
}

It is better to move get, set and normalize functions to Grid prototype. Putting these functions in prototype will increase execution speed and require less memory for creating an instance of Grid because these functions will not be created every time Grid instance is created. Like this:

var Grid = function(w, h){
    this._width = w;
    this._height = h;
    this._grid = new Array((this._width * this._height) | 0);
};
Grid.prototype.get = function(x, y) {
    // get code
};
Grid.prototype.set = function(x, y, item){
    // set code
};
Grid.prototype._normalize = function(x, y){
    // normalize code
};
share|improve this answer
    
Come to think of it, adding this._width to x right before taking the modulo with this._width doesn't actually do anything, right? So: x = (this._width) + x) % this._width is the same as x %= this._width. –  Erty Oct 10 '13 at 8:42
    
It makes sense, when x < 0... But separating behavior for positive and negative values by if statements is also ok. –  Just_Mad Oct 10 '13 at 8:48
    
Ohh right, 'cause it's negative. Thank you :D –  Erty Oct 10 '13 at 8:51

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.