新米なので、こういったことを意識するといいよというアドバイスがいただけるとありがたいです。
プログラムはじめたてのころ
まず、プログラミングを学び始めて、ある程度の計算やSQLをかけるようになったとき、困ったことが、
・メソッドや変数名の命名
・何がいいコード何だろうと、そもそもいいコードの基準がわからなかった
そこで、「リーダブルコード」やほかのQiita記事を読み、命名や循環的複雑度(ifやforなどの階層の深さでコードの複雑度を判断)を下げたり、可読性を上げたり、共通化する(カプセル化)方法を学びました。循環的複雑度を下げるときに、そのコード自体の処理を見直す、インターフェースを利用などがあります。「リーダブルコード」のコード指針としては、
・他人が見て、最短で理解できるコード
と書いてありました。最短で理解できるコードが書ければ、コードをレビューする人、コードを改修する人の時間が削減されるなど、多くの利をもたらします。それと、そういったコードが書ければ、自分のテンションもあがりますね。
そして、僕は自分のコードを書く指針として、「他人が見て、最短で理解できるコード」を設定することにしました。
最近気にしていること
前々から、コードを書いて自動テストを行いたいと思っていました。(Laravelでは、PHPUnitやDusk)
理由としては、デバッグが楽になったり、後々の機能追加の際も、既存の機能のテストに時間は取られないためです。
そこで、既存プロジェクトにテストを入れようとしました。テストをいれることを考慮されていないため、そのプロジェクトにテストをいれるのは、すごく大変で、テストコードの可読性も低い(途中のデータ成型などが多く、肥大したテストコード)ものになってしまいました。なので、自分で作っていて、これってテスト仕様書を書いて、手で実行したほうがめちゃくちゃ早くないかなと思っていました。
そんな時、Laravelでバリデーションをするときによく使うFormRequestを、自動で作れて、そのクラスに自動テスト(入力値を同値分割と境界値チェック)を自動で作成してくれるツールをつくってみてと上司の方にお話をいただきました。自動FormRequest作成
それを作ったときに、Laravelのバリデーションルールに対して、テストしているだけなように感じました。しかし、前回テストを入れた時と違い、こういった部分的な自動テストはいいんじゃないかなと感じました。テストをいれるにしても、基準が必要だと感じました。
そして、今回新しいプロジェクトに入ったときに自動テストしやすくすると思いを抱き、作成していきました。
まず、テストコードを入れるためには、テストコードを入れやすいようにする必要がありました。肥大化したテストコードを書かないために、僕のいいコードの指針にテストを入れやすいコードを書くというのが追加されました。
Laravelでは、コンストラクタインジェクションを使い、そのコンストラクタで指定されているものを利用するときに、mock化したものを利用できることは知っていたので、導入しました。しかし、テストを入れやすいコードって、具体的にこれ以外にしらないなぁと感じました。僕の指針の「他人が見て、最短で理解できるコード」これは、テストを入れる人の時間を削減することはできるけど、テストコード自体の肥大化を抑えるには、至らないのかなと思いました。
可読性 ≒ テストを入れやすいコード
そんな中、機能を作り終わり、僕よりはるかに高い技術を持つ方々に、今回苦労した数字を扱う関係の機能のコードをレビューしていただけることになった。そこで指摘いただいたのが、
・カプセル化があまい
・テストを入れやすいように、持っていきたいなら、カプセル化をするときの意識を変えたほうがよい
というアドバイスを頂けました。
カプセル化があまい
これは、僕のカプセル化したメソッドが、まだ全然さらにカプセル化できる状態で、メソッドに対して単体テストがいれられない状況。そもそものカプセル化があまい。単体テストできるレベルまで、カプセル化したほうがいい。
テストを入れやすいように、持っていきたいなら、カプセル化をするときの意識を変えたほうがよい
これは、僕がカプセル化しているメソッドに渡している引数とメソッドの中身が悪かった。
例えば、計算するメソッドがあったとして、
public function calc()
{
$list = DB::table('numbers')->select('number')->get();
・・・・・・・
}
public function calc($list)
{
・・・・・・・
}
という二つのメソッドがあったとき、上のメソッドは、メソッド内でDBからデータを取得して、そのあと処理している。
下のメソッドは、DBからデータを取得したものを引数としてもらって処理している。テストを入れることを考えれば、データベースの値はなるべく使わないほうが簡単なテストコードになる(DB操作がない分)し、より単体テストといえると思う。
上のメソッドはテストを入れる際に、DBに入れる値を用意して、DBに入れて、このメソッドを呼び出して、返り値をチェックする。
下のメソッドなら、値を用意して、メソッドを呼び出して、返り値をチェックする。
なるほどーーーと思った。これは僕のコード指針に速攻追加した。最上位メソッド以外は、なるべくDBの値(動的値)を呼び出さないようにして、引数として受け取り、テストを入れる際はDB操作をしなくて済むようにしたほうがいいと感じた。また、引数に多次元配列などを指定する場合でも、テストをするときにデータを作るのが大変なので、やめたほうがいいとわかった。そうならないように設計することが重要だった。今までは、コードがあって、テストがあるって考え方だったけど、テストを考えて、コードを書くほうがよりよいと気づけた。
さらに、テストを入れる基準に、数字関係の機能を作った際は、ぜひ入れたほうがいいなと思った。お金の計算なんかは自動テストを入れることですごく楽できるし、正確にデバッグできるし、また、自動テストをいれることを意識することがやはり、いいコードにつながるんだな(カプセル化など)と感じた。
まとめ
僕の現在のコード指針
・他人が見て、最短で理解できるコード (命名・循環的複雑度・共通化などなど)
・テストを入れやすいコードを書く
僕の今の基準のテストを入れやすいコードとは、単体テストが入れられるレベルのカプセル化が意識されたコードであり、動的な値は引数として受け取っているメソッドが作成されていることと後は、コンストラクタインジェクションが使われているプロジェクト
また、いろいろ気づくことがあったらルールを追加していきたい。
新米なので、こういったことを意識するといいよというアドバイスがいただけるとありがたいです。