Javascriptで配列に追加しない場合、単純なチェックが行われます

Question

この関数は、選択された行のidを取得し、配列内にあるかどうかを確認します。それを配列に追加し、それ以外の場合は配列から削除します。問題は、私はイベントの順序を正しく得ることができないということです。ループは途切れているわけではありませんが、ブレークを削除すると、イメージが変わる（チェックボックス）が働いても配列は間違っています。

関数の中にそれを入れずに、この機能のdeleteString = [];外を宣言にもかかわらず、deleteString.push(orderId);の呼び出しが失敗した理由を私も理解していない

それはどのように大きなにかかわらず、最初の実行時に、問題を明らかに思えますチェックが一致するかどうかにかかわらず、配列は実行されません。だからおそらく、ループが見つかった/見つからないという結果を使う前に、ループが完了するまで待つ必要があります。

function passSelection(orderId) { 
    // check if empty 
    if (deleteString.length == 0) { 
    // turn into array 
    deleteString = []; 
    // first entry 
    deleteString.push(orderId); 
    // mark this row as checked 
    $("#"+"select-box-"+orderId).attr('src', 'images/red-checked.png'); 
    } 
    else { 
    // not the first order 
    // check if already in array 
    // get length of array 
    var delStrLen = deleteString.length; 
    // loop through array 
    for (var i = 0; i < delStrLen; i++) { 
     if (deleteString[i] == orderId) { 
     // match found, remove from deleteString array 
     deleteString.splice(i, 1); 
     // update the row 
     $("#"+"select-box-"+orderId).attr('src', 'images/unchecked.png'); 
     break; 
     } 
     else { 
     // not in the array 
     // add to array 
     deleteString.push(orderId); 
     // update row 
     $("#"+"select-box-"+orderId).attr('src', 'images/red-checked.png'); 
     break; 
     } 
    } 
    } 
} 

Answer 1

あなたは配列の要素の存在をテストするためのindexOfを使用するべきです。要素がそう を見つけることができない場合 のindexOfは、ここで私はあなたが配列かどうかをチェックするために最初の条件を必要としませんリファクタリングと実際

var deleteString = []; 
function passSelection(orderId) { 
    // check if empty 
    let orderPos = deleteString.indexOf(orderId); 
    if (orderPos == -1) { 
    // first entry 
    deleteString.push(orderId); 
    // mark this row as checked 
    $("#"+"select-box-"+orderId).attr('src', 'images/red-checked.png'); 
    } 
    else { 

    // match found, remove from deleteString array 
    deleteString.splice(orderPos, 1); 
    // update the row 
    $("#"+"select-box-"+orderId).attr('src', 'images/unchecked.png'); 
    } 
} 

Answer 2

最初にforループ内の要素を削除することは、トラバースしている配列を操作している場合にはお勧めできません。配列内の項目を検索するには、indexOfメソッドを使用する必要があります。

function passSelection(orderId) { 
    // check if empty 
    if (deleteString.length == 0) { 
    // turn into array 
    deleteString = []; 
    // first entry 
    deleteString.push(orderId); 
    // mark this row as checked 
    $("#"+"select-box-"+orderId).attr('src', 'images/red-checked.png'); 
    } 
    else { 
    // not the first order 
    // check if already in array 
    // get length of array 
    var delStrLen = deleteString.length; 
    // loop through array 
    var index = deleteString.indexOf(orderId); 
     if (index > -1) { 
     // match found, remove from deleteString array 
     deleteString.splice(index, 1); 
     // update the row 
     $("#"+"select-box-"+orderId).attr('src', 'images/unchecked.png'); 
     break; 
     } 
     else { 
     // not in the array 
     // add to array 
     deleteString.push(orderId); 
     // update row 
     $("#"+"select-box-"+orderId).attr('src', 'images/red-checked.png'); 
     break; 
     } 
    } 
} 

Answer 3

問題はループ内に要素を追加することです。ループ内でその存在を確認し、項目を一度追加/削除する必要があります。また、期待どおりにループが発生しませんし、それが動作します、空の配列のための特別なチェックは必要ありません。

function passSelection(orderId) { 
    var isFound = false; 

    for (var i = 0; i < deleteString.length; i++) { 
     if (deleteString[i] == orderId) { 
      isFound = true; 
     } 
    } 

    if (isFound) { 
     deleteString.splice(i, 1); 
     $("#" + "select-box-" + orderId).attr('src', 'images/unchecked.png'); 
    } else { 
     deleteString.push(orderId); 
     $("#" + "select-box-" + orderId).attr('src', 'images/red-checked.png'); 
    } 
} 

あなたのアプローチは、一般的にはあまりにも困難です。単にSetオブジェクトを使用して、この結果をより良い、より便利で実演的な方法で達成することができます。
SetアルゴリズムはO(1)アルゴリズムであり、配列を横断するアルゴリズムはO(N)アルゴリズムであるため、は1000倍遅くなる可能性があります。

var set = new Set(); 
function passSelection(orderId) { 
    if (set.has(orderId)) { 
     $("#"+"select-box-"+orderId).attr('src', 'images/unchecked.png'); 
     set.delete(orderId); 
    } else { 
     $("#"+"select-box-"+orderId).attr('src', 'images/checked.png'); 
     set.add(orderId); 
    } 
} 

あなたは事前のECMAScript 6つのブラウザがサポートし、その後、あなたは同じようにオブジェクトのプロパティを利用できる必要がある場合は、次の

var set = {}; 
function passSelection(orderId) { 
    if (set[orderId]) { 
     $("#"+"select-box-"+orderId).attr('src', 'images/unchecked.png'); 
     delete set[orderId]; 
    } else { 
     $("#"+"select-box-"+orderId).attr('src', 'images/checked.png'); 
     set[orderId] = true; 
    } 
} 

Answer 4

をテストして、あなたのコードは、検索項目または-1の位置を返します。長さはゼロです。他の条件からブレークを削除します。 dasoroとして、indexOfメソッドを使用することをお勧めします。ループの代わりに。また、すべての人があなたのロジックを簡単に理解できるように、完全なjsfiddleサンプルを投稿してください。

function passSelection(orderId) { 
    if (!deleteString) deleteString = []; 

    var index = deleteString.indexOf(orderId); 
    if (index !== -1) { 
    deleteString.splice(index, 1); 
    $("#"+"select-box-"+orderId).attr('src', 'images/unchecked.png'); 
    } else { 
    deleteString.push(orderId); 
    $("#"+"select-box-"+orderId).attr('src', 'images/red-checked.png'); 
} 

Answer 5

配列が存在するかどうかを確認しないように初期化してください。

var arr = [ 4,5,6,1,23 ]; 
function remove(orderId) { 
     var src; 
     var index; 
     index = arr.indexOf(orderId); 
     if (index === -1) { 
       arr.push(orderId); 
       src = 'images/unchecked.png'; 
     } else { 
       arr.splice(1,index); 
       src = 'images/red-checked.png'; 
     } 
     $("#"+"select-box-"+orderId).attr("src",src); 
} 
// Test item which is in the array (it should be removed) 
remove(5); 
console.log(arr); 
// Test item which is not in the array (it should be added) 
remove(24); 
console.log(arr);