ある日のコードレビューコメントにて

新しくインスタンスを作って返さないとSide Effectが発生するので修正しました。
今回のコードだと発生しても問題ないですが基本的にSide Effectは発生させない方がバグになりにくくて良いです。
もし発生させる場合はコメントで明記した方が良いです。

エンジニア6年目わい：Side Effectって知らん...何それ...怖...
ってなったので自分の理解のために簡単に調べたことをまとめました。~~プレパラートハートなので6年もエンジニアやっててそんなことも知らないのかって言わずに生温かい目で見守ってください。~~

Side Effectとは

日本語だと「副作用」と呼ぶようです。

こちらの説明をGoogle先生に翻訳してもらうと以下のようなものを副作用と呼ぶそうです。

副作用とは、関数または式の呼び出しによる状態の変更です。関数または式に副作用を持たせるには、変更する状態がローカルスコープ外である必要があります。たとえば、変更する関数を介してオブジェクトを参照渡し、I/O 操作を実行するなどです。

副作用を抱えている関数や式を呼び出すことで何らかの意図しない変更が発生してしまい、それがバグの温床になりえるんだな〜と解釈しました。

例えば以下のようなコードは副作用が起きているコードになります。（コードレビューでもらったコメントも似たような処理してました）

const item = {
  name: 'hoge',
  age: 26
};

const changeName = (item, value) => {
  item.name = value;
  return item
};

const changedItem = changeName(item, 'fuga');

changeNameは受け取ったオブジェクトのnameプロパティの値をvalueの値で更新して返します。
一見これで問題ないじゃん、と思うのですが、ここで以下コンソール出力の結果を見てください。

console.log(item); // {name: "hoge", age: 26}

const changedItem = changeName(item, 'fuga');

console.log(changedItem); // {name: "fuga", age: 26}
console.log(item); // {name: "fuga", age: 26}

はい、元のitemも更新されています。これがSide Effect（副作用）です。ようは関数により意図しない更新がされている状態です。

なんで起きるの

Javascriptのデータ型には

基本型
参照型

の2つがあります。これらの違いについては他の方々が説明してくださっているので省略します。
今回問題となっているobjectは参照型にあたります。参照型では関数などの引数には参照渡し、つまりはメモリ上のアドレスを渡すような動きになります。そのため、元のitemオブジェクトがなぜか更新される、という事象が発生していました。
これを避けるためには受け取ったitemを深いコピーで別アドレスにコピーして、それに対して更新をかける必要があります。

const changeName = (item, value) => {
  const copy = JSON.parse(JSON.stringify(item));
  copy.name = value;
  return copy
};

// 省略

console.log(changedItem); // {name: "fuga", age: 26}
console.log(item); // {name: "hoge", age: 26}

今回のように特にオブジェクトがネストされていなければ以下のようにプロパティを展開して新しいオブジェクトに詰め直してもOKです。

const changeName = (item, value) => {
  const copy = { ...item };
  copy.name = value;
  return copy
};

// 省略

console.log(changedItem); // {name: "fuga", age: 26}
console.log(item); // {name: "hoge", age: 26}

ただし、ネストされている場合にはそのネストされたオブジェクトは浅いコピーになるので注意が必要です。

const item = {
  name: {
    sei: 'hoge',
    mei: 'fuga'
  },
  age: 26
};

const changeName = (item, value) => {
  const copy = { ...item };
  copy.name.sei = value;
  return copy
};

console.log(item); // Object { name: Object { sei: "hoge", mei: "fuga" }, age: 26 }

const changedItem = changeName(item, 'piyo');

console.log(changedItem); // Object { name: Object { sei: "piyo", mei: "fuga" }, age: 26 }
console.log(item); // Object { name: Object { sei: "piyo", mei: "fuga" }, age: 26 }

なんで避けた方がいいの

今回のコードレビューの対象部分は、更新後にすぐそのオブジェクトを呼び出し元に返す動きをしていたので特に問題ありませんでした。しかしそうではなく、後続の処理があり、その処理は変更前の値を使うことを想定されていたらどうでしょう？明示的に値を変更したわけではないのに、なぜか想定外の値が入っているという謎の現象に直面することになります。
こういったバグの温床をなくすために、副作用はなるべく避ける、もしくはコメントとして残して他の人が改修などをした際に予期しない挙動をしないようにする必要があるんだなと思いました。

まとめ

エンジニア6年目のくせに、「今回のコードだと発生しても問題ないですが」を繰り返しまくったせいでこの辺りの仕組みを意識することなく使ってるなと反省したので、今後もこういうことは備忘録的にまとめていきたいなと思いました。

他にも6年目ならこの辺知っとけよ、ってことあればコメントに優しく書いてくれると喜びます。