2012-04-02 34 views
4

私は2つの配列を持っています。ユーザーがプロダクトを追加すると、そのプロダクトをProductArrayに挿入します。それらの製品が削除された場合は、ProductArrayRemove配列に追加し、製品配列から削除します。 (。。私たちは、削除された製品だけでなく、追加された製品を知っている必要があります。これは、冗長性を必要とする)配列から項目を削除する最も効率的な方法は?

ProductArray = JSON.parse(ProductArray); 
ProductArrayRemove = JSON.parse(ProductArrayRemove); 

Iアレイに追加し、アイテムとき、私は単純に次の操作を行います。

ProductArray.push(ItemID); 
ProductArrayRemove.push(ItemID); 

しかし、私はそれを削除するとき、私はこれをしなければならない。

var len = ProductArray.length; 
for (i = 0; i < len; i++) { 
    ProductID = ProductArray[i]; 
    if (ProductID == ItemID) { 
     ProductArray.splice(i,1); 
     break; 
    } 
} 

このタスクを達成するためのより良い方法があるはずと思われます。

配列から1つのアイテム(常に整数)を削除するより効率的な方法はありますか?

+0

'VAR I = Array.lengthと;ながら( - I){IF(配列[I] ===アイテムID){array.spliceは、(i、1);ブレーク}}' - 何が間違っていますこの? –

+4

'O(1)'の速度でアクセスする必要がある場合は、配列の代わりにマップを使う必要があります。 –

答えて

1

アイテムを削除するのがすばやく簡単なアイデアは、アイテムの単一のリストを維持し、削除されるかどうかの各アイテムにプロパティを持つことです。

アイテムを削除するには、それをすべてプロパティobj.removed = trueに設定します。

もう一度追加すると、そのプロパティの値が変更されます。

追加されたアイテムだけを反復するには、.removed == trueプロパティを持つものをスキップするだけです。削除された項目だけを反復するには、逆の処理を行います。ここではイテレータのカップルのようになります。

ProductArray.iterateAdded = function(fn) { 
    for (var i = 0; i < this.length; i++) { 
     if (!this[i].removed) { 
      if (fn.call(this, i, this[i]) === false) { 
       return; 
      } 
     } 
    } 
} 

ProductArray.iterateRemoved = function(fn) { 
    for (var i = 0; i < this.length; i++) { 
     if (this[i].removed) { 
      if (fn.call(this, i, this[i]) === false) { 
       return; 
      } 
     } 
    } 
} 

ProductArray.getSubLength = function(removed) { 
    var cnt = 0; 
    for (var i = 0; i < this.length; i++) { 
     if (removed == this[i].removed) { 
      ++cnt; 
     } 
    } 
    return(cnt); 
} 

そして、あなたはこのようにそれらを使用する:あなたはキーがプロダクトID番号sであるオブジェクトを、使用していないのはなぜ

ProductArray.iterateAdded(function(i, val) { 
    // this is the array 
    // i is the index we are iterating 
    // val is the array element we are iterating this[i] 
    // put code here 
}); 
0

は間違われるかもしれませんが、あなたのコードは、単純に(テストと書かれた私の頭の上から、それはそれが、私の知る限りでは動作するはずない

ProductArray.splice(ProductArray.indexOf(ItemID),1); 

と同じことをやっていません

+0

'indexOf'の戻り値が**' ** 'でないかどうかをチェックするのを忘れないでください。最後の要素は切り捨てられます。 –

+0

特定のインデックスを削除する場合は、問題のインデックスであることがよくわからない場合にのみ表示します。 –

0

いいえこのような操作では、削除する要素を見つけて(項目が順序どおりでないと仮定して)、後続の要素を後方に移動するために線形時間がかかります。他の答えが示唆しているように、コードを削除することもできます。

+0

実際のところ、browser *は配列のindexOfをさまざまな方法で最適化することができます。通常のforループは考慮しないため、indexOfを使用する方がカスタムforループよりも効率的です(ネイティブでも疲れすぎてもベンチマークを作る)。 –

+0

iのインクリメントや条件のようなもの?しかし、それは必要なものの前に要素の全リストを横断しなければならないのですか? – wirate

+0

@DavidMulder:それでも配列全体をスキャンする必要があります –

3

あなたは二つのリストの代わりに、整数の配列のオブジェクトを作成し、アイテム削除するdeleteを使用することができます。今すぐ

// removing an item 
function RemoveProduct(ItemID) { 
    delete ProductsAdded[ItemID]; 
    ProductsRemoved[ItemID] = ItemID; 
} 

// adding an item 
function AddProduct(ItemID) { 
    delete ProductsRemoved[ItemID]; 
    ProductsAdded[ItemID] = ItemID; 
} 

を、それが送る必要があるため、これは、サーバー側でいくつかの余分なマングリングを必要としますリストの代わりにjsonエンコードされたハッシュ(またはマップ、または言語に応じてassoc配列)をオフにすることができますが、これは間違いなくもっと速く削除する方法です。

+0

いいえ! *配列*のdeleteは決してオブジェクトリテラル上では使用しないでください。配列上のdeleteは、スプライスではなく値が未定義の領域にアイテムを残します。 –

+2

私のソリューションでは、配列の代わりにオブジェクトリテラルとしてそれらを作成する必要があるので、私は何が間違っているのか分かりません。私は、オブジェクトのインデックスを作成するために配列の構文を使用しますが、それはまだオブジェクトですね。 –

+0

* genius *、少なくともProductObjectに名前を変更できましたが、ProductArrayがサンプルコード内のオブジェクトリテラルであることを意味しているとは限りませんでした。 :P –

0

、およびその値には、必要なメタデータ(たとえば、削除が発生したとき、どのWebページ上にあるかなど)が含まれています。

var added = {432215: {...}, 432676: {...}}; 
var removed = {432215: {...}}; 

あなたが尋ねたように、ちょっとしたことを修正するのが最も効率的な方法でしょう。しかし、あなたの質問は "最も効率的な方法は何ですか?"というのはそれほど妥当ではありません。なぜなら、ユーザーは自分のカートのトップに100個以上のアイテムを持っている可能性は低いからです。あなたがO(N^2)アルゴリズムを持っていたとしても、おそらく1000項目まで問題は見えません。だから、コード化するのが最も簡単な方法を使ってください。

このシステムの設計には潜在的な欠陥があることに注意してください。お客様がアイテムを取り出して後で追加することは合理的です。あなたが追跡しているものによっては、特別なケースが必要な場合があります。

0

私はこれをやっているだろう:

var itemId = 6; 
var products = [1,3,5,6,7]; 
var removedProducts = new Array(); 
removedProducts.push(
    products.splice(products.indexOf(itemId), 1) 
); 
0

ソートは、おそらく驚くほどよく、ほとんどのブラウザに最適化されています。スタックのコンセプトはアセンブリから始まるので、ポップも典型的に非常に高速です。ソート関数を定義できることは本当に強力です。大抵、私はあなたがそれをより使いやすいものにしたいと思う印象を受けますが、これは私が思うところです。だからあなたの場合

Array.prototype.numFindPop = function(id){ 
    this.sort( 
     function(a,b){ 
      if(a === id){ return -1; }//keeps moving id to the right when found 
     } 
    ); 

    return this.pop(); 

} 

//remove 
ProductArrayRemove.push(
    ProductArray.numFindPop(35) 
); 
1

使用、それは価値があるindexOf-アレイ法は、古いと 栄養不良のブラウザにそれを説明する必要があり、余分なコードは次のようなコードで使用するためにそれを持っていますあなた。

var i= ProductArray.indexOf(ProductID); 
if(i> -1) ProductArray.splice(i, 1); 

//このシムは、平均についてです:

if(!Array.prototype.indexOf) Array.prototype.indexOf= function(what, i){ 
    if(!i || typeof i!= 'number') i= 0; 
    var L= this.length; 
    while(i< L){ 
     if(this[i]=== what) return i; 
     ++i; 
    } 
    return -1; 
} 
1

最初のものは、機能を呼び出すと、コードの10行を書くよりも、より効率的であることが多い誤解です。私たちの大部分は、実際に何が機能しているのか、その機能に何本の線があるのか​​は考えていません。例:indexOfは、ループや追加の変数を使用しないと実装できません。

ここでは、pushを使用して要素を追加します。pushは実際に配列の最後に要素を追加します。したがって、配列にもう1つのメモリ位置を追加するのは簡単です。しかし、要素を削除するときには、要素を配列の任意のインデックスから削除します。だから、必要な手順は

  1. ある削除インデックス

明らかにループが必要とされた後、すべての要素のインデックスを調整し、そのインデックスから

  • を要素を削除する要素
  • のインデックスを検索し、ループを使用しても効率が低下しません。

    var len = ProductArray.length; 
    for (i = 0; i < len; i++) { 
        ProductID = ProductArray[i]; 
        if (ProductID == ItemID && i != len-1) { 
         ProductArray[i] = ProductArray[i+1]; 
         ProductArray[i+1] = ItemID; 
        }else if(i == len-1){ 
         ProductArray.length = i; 
        } 
    } 
    

    以下のように何かをあなたがprotoypeに追加して、配列の機能することができます - 私の提案は(すでにループを開始しているとして)spliceを使用して回避し、あなただけのループを使用して要素を削除することです

    Array.prototype.removeItem = function (itemID){ 
        // put the code here replacing "ProductArray" with "this" 
    } 
    
  • 関連する問題