views:

842

answers:

8

I have an array of objects. Each object has a property called name. I want to efficiently remove an object with a particular name from the array. Is this the BEST way?

  private function RemoveSpoke(Name:String):void {
    var Temp:Array=new Array;
    for each (var S:Object in Spokes) {
      if (S.Name!=Name) {
        Temp.push(S);
      }
    }
    Spokes=Temp;
  }
A: 

If you don't mind using the ArrayCollection, which is a wrapper for the Array class, you could do something like this:

private function RemoveSpoke(Name:String, Spokes:Array):Array{
  var ac:ArrayCollection = new ArrayCollection(Spokes);
  for (var i:int=0, imax:int=ac.length; i<imax; i++) {
    if (Spokes[i].hasOwnProperty("Name") && Spokes[i].Name === Name) {
      ac.removeItemAt(i);
      return ac.source;
    }
  }
  return ac.source;
}
Robusto
FYI ArrayCollection is only available in Flex
Quasimondo
@Quasimondo: FYI Flex is one of the tags on the question.
Robusto
Oh true, I totally missed that one.
Quasimondo
What is the point of wrapping it in the ArrayCollection? you could just splice the array directly. Or am I missing something?
sharvey
+1  A: 

I don't have data to back it up but my guess is that array.filter might be the fastest.

James Ward
If you mean something like this:var filterName:String = "something";Spokes = Spokes.filter(function( element:*, index:int, arr:Array):Boolean { return (element.name != filterName);});then I must disappoint you.It's about 5x slower than Joa's and almost 30x slower than mine.
Quasimondo
Haha. You guys cheat! ;) Is array.filter at least the fastest for the loop / iterator based methods?
James Ward
+5  A: 

The fastest way will be this:

function remove(array: Array, name: String): void {
  var n: int = array.length
  while(--n > -1) {
    if(name == array[n].name) {
      array.splice(n, 1)
      return
    }
   }
}

remove([{name: "hi"}], "hi")

You can also remove the return statement if you want to get rid of all alements that match the given predicate.

Joa Ebert
Joa! You rock. But will Apparat be able to make it even faster? :)
James Ward
I allowed myself to compare your "fastest" against mine and come with bad news: removing 500 elements from a 1000 elements array - yours 34 ms, mine 4ms ;-)
Quasimondo
A: 

You could also use ArrayCollection with a filterFunction to get a view into the same Array object

simms2k
+7  A: 

If you are willing to spend some memory on a lookup table this will be pretty fast:

private function remove( data:Array, objectTable:Object, name:String):void {
var index:int = data.indexOf( objectTable[name] );
objectTable[name] = null;
data.splice( index, 1 );
}

The test for this looks like this:

private function test():void{

var lookup:Object = {};
var Spokes:Array = [];
for ( var i:int = 0; i < 1000; i++ )
{
    var obj:Object = { name: (Math.random()*0xffffff).toString(16), someOtherProperty:"blah" };
    if ( lookup[ obj.name ] == null )
    {
        lookup[ obj.name ] = obj;
        Spokes.push( obj );
    }
}

var t:int = getTimer();
for ( var i:int = 0; i < 500; i++ )
{
    var test:Object = Spokes[int(Math.random()*Spokes.length)];
    remove(Spokes,lookup,test.name)
}
trace( getTimer() - t );

}

Quasimondo
One important thing about this solution: it will only work if each "name" is unique. If there are multiple objects with the same name the lookup table will fail, at least if it is built like this.
Quasimondo
interesting... so you basically have two lists with duplicate data... in general would it be better to just use lookup tables and dispense with arrays for these situations? does this work just because the object has a `name` property or the `indexOf` method searches in every property value of the object?
mga
Yes, if you do not need the array for other purposes (like sorting or accessing elements by index) in this case you could just use the lookup table.indexOf finds instances of an object. In this case it does not use "name" at all for the comparison. The name is used as the hash in the lookup table.
Quasimondo
+1. This is clever. On an unrelated and uncalled for rant, you've got to love how people bitch about Object being slow, when it's often the fastest option if used wisely and if it fits in the solution. That said, I'd personally go with just one array, a linear search and splice (and no pre/post increment/decrement trickery in the loop) unless there is an actual and real need for this extra speed.
Juan Pablo Califano
A: 

Here's an efficient function in terms of reusability, allowing you to do more than remove the element. It returns the index, or -1 if not found.

function searchByProp(arr:Array, prop:String, value:Object): int
{
 var item:Object;
 var n: int = arr.length;
 for(var i:int=n;i>0;i--)
 {
  item = arr[i-1];
  if(item.hasOwnProperty(prop))
   if( value == item[prop] )
    return i-1;
 }
 return -1;
}
reelfernandes
It looks like the formatting removed the type of your vector, but I guess it's "Object". I tested it quickly and at least for me using Vector in this case is actually slower. I think if you want to profit from the speed of Vector you should also use a real type and not "Object".
Quasimondo
Thanks Quasi. Did you test using my function? It has an added conditional ( hasOwnProperty ) that would slow it but make sense if you're not sure the object will have the property. To make it faster remove that. But the user asked about efficiency, and this function speaks to that in terms of reusability.
reelfernandes
I tested it with Joa's proposal, so the hasOwnProperty was not the issue here.
Quasimondo
A: 

Perhaps this technique (optimized splice method by CJ's) will further improve the one proposed by Quasimondo:

http://cjcat.blogspot.com/2010/05/stardust-v11-with-fast-array-splicing_21.html

turboHz
+1  A: 

In general you should prefer the old for-loop over "for each" and "for each in" and use Vector if your elements are of the same type. If performance is really important you should consider using a linked list.

Check out Grant Skinners slides http://gskinner.com/talks/quick/ and Jackson Dunstan's Blog for more infos about optimization.

DieTapete