Forums » Impact Engine » Array Prototype Fix

It would be nice if the engine saw a fix for this issue which has been mentioned already (in the private forum):

The way in which erase and random functions have been added to the Array prototype mess with enumeration and make it difficult to use other JavaScript libraries in tandem with Impact.

For example, MIDI.js cannot be used with Impact so long as this is not fixed, because the necessary MIDI.loadPlugin function eventually works its way to these lines:

for (var key in conf.items) {
	self.queue.push(conf.items[key]);
}

The result is that the erase and random functions are pushed into the queue array, breaking the plugin.

The fix appears to be a simple one (courtesy of vincentpiel):

/* impact.js */

Object.defineProperty(Array.prototype,'erase', { value : eraseItemFromArray } );

function eraseItemFromArray(item) {
    var i=this.indexOf(item);
    if (i>=0) this.splice(i, 1);
    return this;
};

Object.defineProperty(Array.prototype,'random', { value : randomItem } );

function randomItem() {
    return this[ 0 | (Math.random() * this.length) ];
};

Hello,

I am not arguing against a fix, but until it is fixed the loop should probably be written such as:

for (var key in conf.items) {
    if(conf.items.hasOwnProperty(key)) {
        self.queue.push(conf.items[key]);
    }
}

See http://yuiblog.com/blog/2006/09/26/for-in-intrigue/ and http://jslint.com/lint.html section 'for in' for more details.

I also created a fiddle to illustrate the problem: http://jsfiddle.net/37Rxd/1/

Fixed in the git repo on your download page.

For the record: I always considered iterating over Arrays with for...in loops "wrong". I remember an older Safari at the time I wrote the first Impact version not supporting it at all and it still has some other pitfalls. I&039;m not arguing against the fix either, I'm arguing against using #for...in for Arrays :)

Awesome, thanks!

Let's be a little serious, here :

The reasons quoted in the link about not using for...in are just not relevant.

for ... in behaves differently from for( ; ; ), one willing to use it has to live with it and open some book or browse MDN.

Adding an issue -by polluting the prototype- could anyway not help at all 'fixing' 'this perfectly legal construct.

...

Could someone post a snippet of the Array fix from the git repo, or perhaps point me in the direction of a tutorial or documentation on how to use the link provide on our download page for cloning.
Thanks

@gxxaxx: Replace these lines in impact.js:

Array.prototype.erase = function(item) {
	for( var i = this.length; i--; ) {
		if( this[i] === item ) {
			this.splice(i, 1);
		}
	}
	return this;
};

Array.prototype.random = function() {
	return this[ Math.floor(Math.random() * this.length) ];
};

...with these lines:

Object.defineProperty(Array.prototype,'erase', { value : eraseItemFromArray } );

function eraseItemFromArray(item) {
    var i=this.indexOf(item);
    if (i>=0) this.splice(i, 1);
    return this;
};

Object.defineProperty(Array.prototype,'random', { value : randomItem } );

function randomItem() {
    return this[ 0 | (Math.random() * this.length) ];
};