yamachu/content.md Secret

## content.md

      
    Raw
  

              content.md
            
          
    私がJavaScriptで書かれたコードを読んで息が詰まるケース…今回はCollection操作についてをご紹介します。
また私はレビューする時cloneしてから臭う変数はリネームをかけて「なんかに使ってそうでどこまで使われてるかわからん」みたいな日本語に置換してからレビューをするレベルで同一スコープの変数管理が苦手な属性を持っています（なんの紹介）。
Array.prototype.sort

最近私がSlackやTwitterで色々言ってることです。
難しいですよね、Array.prototype.sort
Arrayをsortして、その結果を元になにかの処理するみたいなこと、めちゃくちゃやってると思います。
めちゃくちゃ使うメソッドではあるけれども、MDNの返値の所に書かれている

ソートされた結果の配列。ソートは対象配列上で in place に行われることに注意して下さい。コピーされた別の配列が準備されることはありません。

これを意識していますでしょうか。
どういうこと？と思われるかもしれませんが、以下のコードを見ればどういうことかわかるかと思います。
const arr: Array<number> = [5,4,3,2,1];
const sorted: Array<number> = arr.sort();
// sorted => [1,2,3,4,5]
// arr => [1,2,3,4,5]
MDNのドキュメント通りsortedにはarrのsort済みの配列が格納され、結果も望んだものとなっています。
しかしarrはconstで宣言したにも関わらず値が書き換わっています。
Array.prototype.sort は破壊的なsortなのです。
Pythonでいうsortedじゃないsortみたいな振る舞いです。
関数のコールの深さが浅ければこのsortされた元の配列が次にどこで使われているか、それはsortされていても問題ないかが追うことができますが、ある程度の深さになった場合追い切ることは困難になります。
sortが目的なのか、sortされた配列を元に何かしらの作業をすることが目的なのかとか、メモリ制限があってとか、色々と事情によって変わりますが、特に制約はなくsortされた配列を元に何かしらの作業をすることが目的であれば安易にArray.prototype.sort を使わないでくれた方が、私は嬉しいです。
ではどうすればいいか。
色々と方法はありますが、
const arr: Array<number> = [5,4,3,2,1];
const sorted: Array<number> = arr.slice().sort();
// sorted => [1,2,3,4,5]
// arr => [5,4,3,2,1]
みたいにArray.prototype.sliceを使って浅いコピーを作ってからsortをするなどしてみてください。
これで元の配列の要素の並びに関しては守ることができます。
とは言えこれをいつも覚えて実装するのは難しいでしょう。
そういう時のためのlintってことでDisallow mutating objects and arrays (immutable-data)のルールを使ってみると、ルールの設定によってはfilter後のsortも許さんぞみたいなふるまいで使いづらそう…
なのでTypeScriptで書いている時は
const arr: ReadonlyArray<number> = [5,4,3,2,1];
の様に変数を ReadonlyArray<T> として置いてしまうと安全に扱うことができます。
この状態でsortを行おうとすると
Property 'sort' does not exist on type 'readonly number[]'.(2339)

という風に怒ってくれます。
もちろんこれは関数の引数の型にも指定できるので、参照しか行わない、という強い気持ちを示したい時は積極的に使っていきたい型です。
他にもArray.prototype.sortは難しい要素があります。
numberの様な減算によって大小が一意に確定する要素の比較では問題にはなりませんが、文字列などの比較や、オブジェクト内のプロパティで比較を行う際に実装を誤ると意図しない挙動になったりします。
例えば
const compareFunction = (a: string, b: string) => {
    return a > b ? 1 : -1;
}
みたいにbooleanを大小比較の 1, -1 のみにした場合、環境によってはこの結果の順序が一意に定まりません。
MDNに書かれているように
function compare(a, b) {
  if (ある順序の基準において a が b より小) {
    return -1;
  }
  if (その順序の基準において a が b より大) {
    return 1;
  }
  // a は b と等しいはず
  return 0;
}
この様に一意に定まるようにすることが望ましいでしょう。
どういう基準であるかを明確に書いてあるコードを読むと、じゃあ値が同じ場合はこういう振る舞いをするんだなと私が安心して読むことができます。
https://developer.mozilla.org/ja/docs/Web/JavaScript/Reference/Global_Objects/Array/sort#description

compareFunction(a, b) が 0 を返した場合、a と b は互いに変更せず、他のすべての要素に対してソートします。注意: ECMAScript 標準はこの振る舞いを保証していないため、一部のブラウザー (例えば、遅くとも 2003 年以前のバージョンの Mozilla) はこれを遵守していません。

みたいな感じで、古い環境ではこれを遵守してくれる、とは限らないのですが（https://developer.mozilla.org/ja/docs/Web/JavaScript/Reference/Global_Objects/Array/sort#browser_compatibility の stable sortingを参照）。
Array.prototype.map

一日に100回は書くんじゃないかってぐらいめちゃくちゃ便利なArray.prototype.map。
多くの場合は
const arr: Array<number> = [5,4,3,2,1];
const doubled: Array<number> = arr.map(x => x * 2);
// doubled => [10,8,6,4,2]
みたいな感じで戻り値を使うのですが、稀に使わないコードを見ます。
そういう時は大体日本語で変数名を置換しなければいけないブツが転がっています。
そう例えば
const counter: Map<number, number> = new Map()
const arr: Array<number> = [1,1,2,3,5];
arr.map(v => {
    const before = counter.get(v) ?? 0;
    counter.set(v, before + 1);
});
/*
counter =>
  0: {1 => 2}
  1: {2 => 1}
  2: {3 => 1}
  3: {5 => 1}
*/
みたいなmapから外部の変数の状態を書き換えているコードです。
Array.prototype.map のページの一行目に書かれているように

map() メソッドは、与えられた関数を配列のすべての要素に対して呼び出し、その結果からなる新しい配列を生成します。

主目的としては、map内の関数の戻り値から新しい配列を作ることです。
私はmapが書かれていたら値にどういう変換関数をかませるのかなというのを見て、その戻り値が適切な名前で変数として割り当てられているかというのをレビューで見ています。
戻り値がなかった瞬間これは、どこに消えたんだ…………と混乱していまいます。
外部の変数の操作が目的なのであれば、お願いです、Array.prototype.forEachとかfor...ofを使ってください。
Array.prototype.forEachは渡された関数を各要素に実行することが目的の関数のため、読んでいてもいっぱいこれをするんだなーぐらいの能天気さで読むことができます。
先程挙げたコード例でも要素をMapに突っ込むことが最終目的で、それの為に行っていることは…という風に読めばいいという風に思考を変えることが出来るからです。
余談ですが、それでも上記コードのスコープが十分に短くないとforEachを使っていようが詰まるときがあります。
Map という型で宣言されているため、このスコープ内で他に状態を変える操作が行われていないかの調査をする必要があるからです。
これもReadonlyMapとして宣言できればそういう心配もなくなるのですが、それを行うためには
const arr: Array<number> = [1,1,2,3,5];
const counter: ReadonlyMap<number, number> = arr.reduce((prev, curr) => {
    const before = prev.get(curr) ?? 0;
    const next = new Map(prev);
    next.set(curr, before + 1);
    return next;
}, new Map<number, number>());
みたいに流石に大げさすぎんか？みたいなコードにもなってしまうこともあるので、命名やスコープを限っていい感じのコードにしていけると良いと思います。
終わりに

最初辺りにも書いたけれども、色んな制約のもとコードを書いているわけで、今挙げたことをすることが別に悪ってわけではない。
パフォーマンスのためだったり、動作環境の制約だったり、そういうのが色々あることも重々に承知している。
その中で自分が、周りが、読んで意図を汲み取れるコードが書けて、そのコードが価値を出しているのであれば正直なんでもいい。
けれども、もしも、もしも安全なコードに近づけたい、読み手の負荷を減らしたいという思いが少しでもあるのであればこの記事だったりを思い出してくれると嬉しい。
そんな偉そうに語ってるお前はどうなんだ？と問われると
大体Array.prototype.reduceで書いて「ハンマーしか持っていなければすべてが釘のように見える」みたいになっていますテヘペロ
キーワード


破壊的
Immutable
副作用
参照透過性