More than 3 years have passed since last update.

Javascriptのspliceで削除すな

Posted at 2020-01-19

概要

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

OS：Windows 10
言語：Javascript

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データの次のデータにチェックが行われない構造になっていますね。
なので以下のデータで削除漏れが起こります。

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

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

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

以上、駄文でした。