views:

118

answers:

4

I have an array which is part of a small JS game I am working on I need to check (as often as reasonable) that each of the elements in the array haven't left the "stage" or "playground", so I can remove them and save the script load

I have coded the below and was wondering if anyone knew a faster/more efficient way to calculate this. This is run every 50ms (it deals with the movement).

Where bots[i][1] is movement in X and bots[i][2] is movement in Y (mutually exclusive).

for (var i in bots) {
    var left = parseInt($("#" + i).css("left"));
    var top = parseInt($("#" + i).css("top"));
    var nextleft = left + bots[i][1];
    var nexttop = top + bots[i][2];
    if(bots[i][1]>0&&nextleft>=PLAYGROUND_WIDTH) { remove_bot(i); }
    else if(bots[i][1]<0&&nextleft<=-GRID_SIZE) { remove_bot(i); }
    else if(bots[i][2]>0&&nexttop>=PLAYGROUND_HEIGHT) { remove_bot(i); }
    else if(bots[i][2]<0&&nexttop<=-GRID_SIZE) { remove_bot(i); }
    else {
        //alert(nextleft + ":" + nexttop);
        $("#" + i).css("left", ""+(nextleft)+"px");
        $("#" + i).css("top", ""+(nexttop)+"px");
    }
}

On a similar note the remove_bot(i); function is as below, is this correct (I can't splice as it changes all the ID's of the elements in the array.

function remove_bot(i) {
    $("#" + i).remove();
    bots[i] = false;
}

Many thanks for any advice given!

+8  A: 
  1. Cache $("#" + i) in a variable; each time you do this, a new jQuery object is being created.

    var self = $('#' + i); var left = parseInt(self.css("left")); var top = parseInt(self.css("top"));

  2. Cache bots[i] in a variable:

    var current = bots[i]; var nextleft = left + current[1]; var nexttop = top + current[2];

  3. Store (cache) the jQuery object of the DOM element within the bot representation. At the moment it's been created every 50ms.

    What I mean by this is that for every iteration of the loop, you're doing $('#' + i). Every time you call this, jQuery is building a jQuery object of the DOM element. This is far from trivial compared to other aspects of JS. DOM traversal/ manipulation is by far the slowest area of JavaScript.

    As the result of $('#' + i) never changes for each bot, why not store the result within the bot? This way $('#' + i) gets executed once, instead of every 50ms.

    In my example below, I've stored this reference in the element attribute of my Bot objects, but you can add it your bot (i.e in bots[i][3])

  4. Store (cache) the position of the DOM element representing the bot within the bot representation, so the CSS position doesn't have to be calculated all the time.

On a side note, for (.. in ..) should be strictly used for iterating over objects, not arrays. Arrays should be iterated over using for (..;..;..)

Variables are extremely cheap in JavaScript; abuse them.

Here's an implementation I'd choose, which incorporates the suggestions I've made:

function Bot (x, y, movementX, movementY, playground) {
    this.x = x;
    this.y = y;
    this.element = $('<div class="bot"/>').appendTo(playground);
    this.movementX = movementX;
    this.movementY = movementY;
};

Bot.prototype.update = function () {
    this.x += this.movementX,
    this.y += this.movementY;

    if (this.movementX > 0 && this.x >= PLAYGROUP_WIDTH ||
        this.movementX < 0 && this.x <= -GRID_SIZE ||
        this.movementY > 0 && this.y >= PLAYGROUND_HEIGHT ||
        this.movementY < 0 && this.y <= -GRIDSIZE) {
        this.remove();
    } else {
        this.element.css({
            left: this.x,
            right: this.y
        });
    };
};

Bot.prototype.remove = function () {
    this.element.remove();
    // other stuff?
};

var playground = $('#playground');
var bots = [new Bot(0, 0, 1, 1, playground), new Bot(0, 0, 5, -5, playground), new Bot(10, 10, 10, -10, playground)];

setInterval(function () {
    var i = bots.length;

    while (i--) {
        bots[i].update();
    };
}, 50);
Matt
Really useful advice, I shall post the updated code shortly. However I don't really understand what you mean by #3 could you advise further? Many thanks,
Pez Cuckow
@PezCuckow: I've expanded my explanation for #3
Matt
+1  A: 

You're using parseInt. As far as I know, a bitwise OR 0 is faster than parseInt. So you could write

var left = $("#" + i).css("left") | 0;

instead.

Furthermore, I wouldn't make use of jQuery functions to obtain values like these every 50 ms, as there's always a bit more overhead when using those (the $ function has to parse its arguments, etc.). Just use native JavaScript functions to optimize these lines. Moreover, with your code, the element with id i has to be retrieved several times. Store those elements in a variable:

var item = document.getElementById(i);
var iStyle = item.style;
var left = iStyle.left;
…

(Please note that I'm not a jQuery expert, so I'm not 100% sure this does the same.)

Moreover, decrementing while loops are faster than for loops (reference). If there's no problem with looping through the elements in reverse order, you could rewrite your code to

var i = bots.length;
while (i--) {
    …
}
Marcel Korpel
Re your use of while, I need to use the actual ID of the element in the array, as I delete/remove some it can't just be a loop with increasing or decreasing values.Otherwise, fantastic advice!
Pez Cuckow
A: 

Take a look here for a great comparison of different looping techniques in javascript.

Using for...in has poor performance and isn't recommended on arrays. An alternative to looping backwards and still using a for loop is to cache the length so you don't look it up with each iteration. Something like this:

for(var i, len = bots.length; i < len; i++) { ... }

But there are MANY different ways, as shown in the link above and you might want to test several with your actual application to see what works best in your case.

Tauren
A: 

Use offset() or position() depending on if you need coordinates relative to the document or the parent. position() is most likely faster since browsers are efficient at finding offsets relative to the parent. There's no need for parsing the CSS. You also don't need the left and top variables since you only use them once. It may not be as readable but you're going for efficiency:

var left = $("#" + i).position().left + bots[i][1];
var top = $("#" + i).position().top + bots[i][2];
Bob