自分が「読み解きづらいな」と思ったコードに対して、なぜ読み解きづらいと思ったのかを考察し、
また、そうならないようにどうしたいかを考えたので書いてみる。
個人的な考えだし、エンジニアレベルとか状況とかで変わると思うし、僕が見えてない事情とかもあると思うけど、一旦そういうのは無視して書いてる。
フックベースで処理が組まれていて、フックの種類が多くネストが深い
イベント駆動みたいに「なんらかをフックして次の処理が実行される」みたいな機能をふんだんに使って処理を組まれていると、次なにが実行されるのかが全然わからない。
たとえばActive Recordは以下のコールバックがある。
before_validation after_validation before_save around_save before_create around_create after_create after_save after_commit/after_rollback before_validation after_validation before_save around_save before_update around_update after_update after_save after_commit/after_rollback before_destroy around_destroy after_destroy after_commit/after_rollback
これらを全て確認しないと処理を追えない。
実行順序も意識しておく必要もある。
さらにそのコールバック処理の中で別のActive Recordが使われていたりすると、そのコールバックも把握して追っていかないといけない。。
これらのコールバックを駆使して組まれた処理は
「someObject.save()
って実行すると上手く処理されるけど、どういう処理が行われているのかよくわからん。処理を追うのも大変。。」
という感じになる。
対策:
フックに過剰に頼らない設計にする。(フックに向いている処理ってのはあると思う)
何が実行されるのかを外部が制御してる
「実行メソッド名が外部から渡ってきてて、そのメソッドを実行する」みたいなやつ。
実際に外部からデータもらわないと何が実行されるかわからん。
大体は「内部処理を外からも汎用的に使えるようにした」みたいなやつがこういう感じになってる気がする。
対策:
外部が内部の実行メソッド名を知っているという状態がそもそもおかしい。
何を実行するのかは内部で制御するようにする。
メソッドだけ見ても何をしているのかよくわからない
こんな感じのやつ。
calculator = New Calculator(user) calculator.execute() price = calculator.price
priceがどういうデータからどういう計算をされた値なのか全然わからん。
ユーザー情報を渡してなんらかの計算をしているっぽい雰囲気しかわからん。
executeメソッドの中身を見ると、データベースから色々データを取得して、なんらかの計算をするロジックが長々と書かれていたりする。
対策:
引数と返り値を使って、なんのデータがどういうデータになるのか解るように表現する。
不必要にに広範囲のデータを渡さないようにする。
単体で動かせない
コードリーディングで挙動を読み解けない場合、実行してみて動きを見たいが依存が多く動かせない。
巨大なダミーデータの作成が必要になって、どういうデータが必要なのかを理解するためにコードリーディングが必要になって、読み解けなくて、、
対策:
依存をできるだけ減らしていつでもどこでも単体で動かせるようにする。
実現したいことに対して実現するための構成要素がなんか多い
1つのことを実現するために5個くらいclassが存在したり、5個くらいテーブルがあって絡み合ってる。 その思想を読み解けず、なんのためにこれだけの要素があるのかわからないので処理が追いにくい。
対策:
それを実現するのに一番自然に思い浮かぶロジックをそのまま表現できるようなコードにする。
今必要ではない念の為の機能のために不要にコードを太らせないようにする。(使われてない機能だから余計読み解きにくい。。)