Javascriptのspliceで削除すな

概要

Backendで動くJavascriptのコードを引き継いだ際に、ちょっとやばそうなミスをしていたので共有します。

環境

OS：Windows 10
言語：Javascript

splice関数？

Javascriptの配列標準関数。
大体の使い方は以下を参照。
spliceは切り取るだけのメソッドではなかった

問題箇所

問題の箇所は以下。
特定の形のエラーデータをspliceで削除しようとしていた。

// ErrorData削除処理
for(var i=0; i<array.length; i++){
  if(checkErrorData(array[i]===true)){
    array.splice(i, 1);  // i番目の要素を削除
  }
}

一見問題なさげに見えるけど大問題。
splice関数は配列のインデックスをめちゃくちゃにするので一連のループ処理で何度も実行するとえらいこっちゃになる。

ちょっと掘り下げ

実例で考える。

Errorデータを削除するため、i=3時に削除が実行されます。

この時、削除された段階で配列が作り替えられています。forループのカウンタiはそのままインクリメントされるため、元々index=4だった要素に対してErrorチェックが行われません。

問題まとめ

Errorデータの次のデータにチェックが行われない構造になっていますね。
なので以下のデータで削除漏れが起こります。

対策

1. splice関数を使ったお姉さんに文句をいいましょう
2. 配列から要素を削除する際はfilter関数を使いましょう

• filter関数は以下を参考

【JavaScript入門】filterで配列のデータを抽出する方法

削除というよりはErrorデータ抜きの新しい配列を代入する感じです。
速度的にも圧倒的にこっちの方が早いですね。
以下に書き換えました。

// ErrorData削除処理
array = array.filter( function (item) {
  return checkErrorData(item) === false;
})

callback関数でitem（配列の各要素）をチェックして、正常なデータの時だけtrue値を返す感じです。

以上、駄文でした。