やる気がストロングZERO

やる気のストロングスタイル

「良いコード」と「良くないコード」の特徴

自分が思ってる「良いコード」と「良くないコード」の特徴を書きなぐってみる。

関数

良い

引数と返り値を見て、何をしているのかなんとなくわかる。

c = add(a, b)

# aとbが加算されてるっぽい

良くない

引数と返り値を見ても、何をしているのかよくわからない。

c = add()

# なにかを加算してるらしいけど、何が加算されているのか、、(内部でdbからデータを取得して加算して返してたりする)

どういうこと?

処理の詳細を隠して一言(関数名)で表される事で抽象化を行って理解しやすいコードにするのが関数の一つの役目。
中身を見ないと理解できない関数は、中身が隠蔽されている分理解しづらくなっているだけなので良くない。

関数は外から見てやっていることが、どういう実装なのかは考えずに解ることが大事だとおもう。

カスタマイズ

ある一連の処理にて、実験的に一部の処理をスキップさせたい。

良い

単に該当部分をコメントアウトし、値が帰ってきたことにすれば意図通りに実行できる。

良くない

どこが該当処理なのかよくわからない。
ここだと思う部分をコメントアウトして実行すると、別の場所でエラーが出る。
見てみるとコメントアウトした処理が実行されたときにDBにデータが挿入されており、その値がないので後続処理でエラーになっていた。

どういうこと?

良いコードは処理と処理が独立していて、ある処理が別の処理に直接影響していることがない。(処理の結果値を次の処理がもらうことはあるが、値が必要なだけで処理自体が必要なわけではない)
だから、次の処理のために仮の値を用意してやることで簡単にスキップすることができる。

良くないコードは、ある処理の副作用が別の処理に依存されていたりして、処理を書き換えることが簡単にはできない。中を調べて何が行われているかを知り、別の処理に影響がでないように変更してやる必要がある。

仕様確認が簡単にできる

あるapiのバリデーションルールを知りたい。

良い

そのapiのバリデーション定義見てすぐ理解できる。

良くない

そのapiのバリデーション定義を見ると、複数のapiから共有して使われており、多数の条件分岐が存在していた。リクエストパラメータによる分岐もあり、どういう値が渡ってくるのかも把握しないといけない(しかしそれは今回の対象apiでは渡ってこないものなのかもしれない)。 先頭から条件分岐を追っていかないとルールが把握できない。。

どういうこと?

設計されずにその場しのぎでコードがいじられると陥る。
アドホックにif分岐で対応され続けた結果、論理的な構造を持たないコードの塊になってしまっている状態。

リファクタリング

メソッドの定義位置に違和感が出てきたので別のクラスにメソッドを移動させたい。

良い

メソッド定義場所を移動させ、呼び出し元を書き換えて、少し調整すれば移動できた。

良くない

メソッド定義場所を移動させ、呼び出し元を書き換えて、少し調整したがエラーがでた。
調べると内部でdbデータの読み書きを行っており、そのためにライブラリを使っているが、移動先にはライブラリがインポートされていない。
移動先にライブラリをインポートして、DB接続初期化処理を用意してやらないといけない??

どういう事?

何らかの処理メソッドなのに、内部でdbアクセスまでやってる(もしくは別のオブジェクトに副作用を与えてる)から、保存ライブラリや別オブジェクトに依存した形になってる。

計算して結果を返し、別オブジェクトへの干渉は別途やっていれば(つまり一つのことだけ行うようにしていれば)メソッド定義場所の変更は容易なはず。

テーブル定義

良い

テーブル構造とデータをみれば、コードを追わなくても状態が把握できる。 どのようにデータを変更すればどのような状態になるのか、コードを追わなくても解る

良くない

テーブル構造とデータの中にフラグが多数あり、どのフラグがどうなっているかによって、データの見方が複雑に変化するので理解が追いつかない。
「こういうデータは存在しえない」とか「typeが○○のときoptionには××の情報が入っている」とか、データの見方に暗黙のルールがたくさんある。

どういうこと?

  • テーブル設計が詰めきれておらず、コード側で補完してる。(だから処理と組み合わせて見ないと理解できない。)
  • 処理のためのデータが存在してでそれがノイズになってる。(それはコードを見ないとどのように使われているのかわからない)

こういうのは大抵コード側も複雑な事になってる。。

導出できる値の扱い方

例えば「消費税」をどのように扱うか?

良い

算出メソッドが定義されてて、必要なとき実行し定価から算出されるようになっている。

良くない

算出した消費税額を保持していて、定価が変化(値引きなど)するたび再計算して保持し直している。

どういうこと?

こういう事をする動機としては「一旦計算した値を保持することで何度も計算せずにすむ」とかなのかなと思うが、
だいたい管理できなくなって事あるごとに再計算処理を実行するような感じに陥ってたりする。

例)
* ここで定価を変更した、このあと消費税の再計算を実行しているかよくわからず不安だから再計算を実行しておく。
* ここで消費税が必要だけど、どこかで消費税計算実行が漏れていたら怖いから再計算処理を実行しておく。

結果、なんどもなんども消費税の再計算が不要に行われてたりする。(何度も計算せずに済むはずだったのに。。)

つまり、データの整合性を自力で取っていかないといけないので複雑正が増している。