結論
- コードレビューをして
- コードの綺麗さと経験年数は比例しない
- コードは綺麗にしてからマージして
- 「品質よりも速度優先」は品質落として速度も落としている
- リファクタリングは誰でもできる
上記を理解できているのであれば、次項以降を読む必要はないです。
偏見ですが、スリッパ揃えない人やトイレの後に手洗わない人は嫌でも見てください。
スリッパを揃えられるからといって、綺麗なコードが書けるわけではないですが、綺麗なコードを書く上で必要なものは、他人のことを意識するのが重要だと思っています。スリッパを揃えて次の人が使いやすいようにしようとか、用を足した後に手を洗わずあちこち触ったら、そこに触れた人の手が汚れるとか。そういったことを日頃から考えるのが大事だと思います。
体験談
業務で他人のコードを見る機会が結構あります。
主にプロジェクトから離任した人や、今忙しいからという人達が書いたコードのバグ対応や仕様変更対応、Phase2対応など。
忙しいって自分も忙しいけど
綺麗なコードよりもクソコードを見るほうが多いです。クソコードは一度書かれたら最後。担当者によってリファクタリングされることはありません。
クソコードとは
よくあるクソコード例
- スコープが広い
- 変数名が雑
- 条件分岐などでいきなり出てくる謎の数値(マジックナンバー)
- ネストが深い
他にもありますが、一貫して言えるのは下記の通り。
「他人が見た時に何をしてるのか」わかりにくい
上記には該当してないですが、下記もクソコードです。
- 同じ処理をメソッド化せずに何度も記載している
※最近はあまり見かけないですが、レガシーシステムには結構あると思います。
クソコードのなにが問題なのか
まず業務で実装されたコードは、「実装担当者しか触らない」ということはないです。
自分と同じようにいつかは実装担当者以外の人に該当の機能の引継ぎがやってきます。
そのコードがクソだったときに下記のようなことが発生します。
- コード品質を意識したものと比べ、処理の理解に時間がかかる
- 変更を入れると別の複数箇所に影響がでるのがわかった
- 変更により意図していない箇所に影響がでてしまった
- やる気が出ない
など
要するにメンテに不要な時間がとられます。
メンテではないですが、実装担当者が自身の書いたクソコードを理解できず、テストパターンを網羅できてないというのはよくあります。なので機能を引き継いだ時点でバグってるのはあたり前です。ハッピーパス通ってないときもある。アンハッピー
それならせめてコードのマージ前にレビューを設けることで、可読性のチェックができ、クソコードのマージを阻止できると思います。
レビュアーのやるべきこと
レビュアーがやるべきことは下記の通り
- そのコードが見やすいか
- 何の処理なのかざっくり把握する
- 読みにくいなら指摘する
レビュアーはコードが設計書通りになっているかまでは見なくていいです。
コード内容が設計書通りかなんてレビュアーの所掌範囲外なのと、そもそもまともにテストにしてないものをレビューに出すこと自体がおかしいです。
実装担当者が自分よりも年上、経験年数が上とかで指摘しづらいというのがあるかもしれないですが、気にする必要はありません。
コードの綺麗さと経験年数は比例しない
むしろ指摘しづらいのを理由にザルレビューしてたら、そのクソコードのメンテ担当が自分になったとき、自分の首を絞めることになります。
経験年数長くてもクソコードを書く人は書くので、新人ではないからレビュー不要というのは、言い訳にすらなってないです。
「自分のコードが他人からどう見えているのか」ということに意識が向かない限り、
綺麗なコードを書くのは難しいと思います。
自分のコードの見え方はレビュー含め、他人のコードを見ることで、自分が普段書いてる書き方は普通に読めるなとか、読みにくいなとか気づきが得られると思います。
レビューイ(実装担当者)のやるべきこと
プログラムが期待通りに動いたらレビューしてもらうではなく、コードをステージングしておくか、コミットするなりして現時点にいつでも戻せるようにしておきます。その後リファクタリングをし、再度期待通りに動くことを確認できたらレビュー依頼をだします。
「レビューがないプロジェクトならリファクタリングしなくてもいい」というわけではないです。伝えたい内容の意図としては下記のとおりです。
コードは綺麗にしてからマージして
クソコードのメリット
クソコードはクソでしかないとわかったと思います。
ではクソコードのデメリットに見合うメリットはないのかと思った人もいるはず。
全然思いつかなかったのですが、1個ありました。
SEやってれば一度は聞いているであろう言葉
- 品質よりも速度優先で
品質を落とすことで作業速度を上げるというメリットがあります。
指示した人のイメージの中では。
結論から書くとメリットはないです。
確か「レガシーコードからの脱却」という本で出てきますが、要件・設計時のバグ修正のコストを1としたとき、リリース後にバグ対応をすると最大で100倍のコストがかかる というのが記載されています。詳細はグリッドのとおり。
開発手法は書いてなかったと思いますが、コストが重すぎるのでおそらくウォーターフォールだと思います。
| フェーズ | 相対コスト |
|---|---|
| 要求・設計 | 1 |
| 開発 | 1~6 |
| テスト | 1~15 |
| UAT・プレリリース | 1~30 |
| リリース後 | 50~100 |
品質重視であれば事前に防げたであろうバグをテスト時点で発見して直したとして、
運が良くてコスト1倍、悪くて15倍かかる可能性があります。
リリースしてしまったのなら最大100倍。
下記は憶測で記載しています。
品質を落としている以上、品質重視よりバグが出るのは見込んでいると思いますが、この修正コストを補えるだけの速度で実装を進めるのはおそらく不可。
クソコード唯一のメリットの実態は下記の通り。
「品質よりも速度優先」は品質落として速度も落としている
そもそもウォーターフォールでの修正コストが何倍かかるかとか置いといて、「ウォーターフォールは後戻りのコストがでかい」というのはよく聞く話だと思います。それを知っていながら、あえて後戻り前提で進めるのは自分が良ればいい、今が良ければいいというのがちらついて見えますね。
息抜き
コード品質の話をすると、ユニットテストの話がよくあがります。
ユニットテストでは「コードが正しく動くか」しか見れないので、ユニットテストでわかるのはプログラムとしての品質であってコード自体の品質ではないと常々思っています。
コード自体の品質の指標をコードメトリクスといい、クソコードが何をしているのかわかりにくいというのは認知的負荷が高いと表現されています。
関連ワードは下記のようなものがあります。
- 循環的複雑度
- 認知的複雑度
- 短期記憶
- 長期記憶
- ワーキングメモリ
- チャンク
この辺は「良いコード悪いコードで学ぶ設計入門」や「良いコードの道しるべ」という本で紹介されているので、とりあえず内容を確認したいというのであれば上記2点がおすすめです。がっつり学習したいなら「プログラマー脳」という本がおすすめですが、本屋やAmazonで長いこと見てないです。
綺麗なコードを書くために下記のようなものがあげられます。
- スコープは狭く
- 変数名はわかりやすく
- 1メソッドに関連のない処理を入れない
- 1クラスに関連処理をまとめる
など
これらの対応により、なぜ見やするくなるのかという本質的な部分に触れる内容なので、個人的にはかなり面白い内容となっています。今度気が向いたら記事書こうかなと思ってます。
息抜き終了
息抜きの前の項目で何が書いてあったのか、はっきり覚えてますか。
たぶん息抜きの内容に記憶がほとんど持ってかれたのではないでしょうか。
記憶が持ってかれている前提で記載しますが、これは短期記憶云々の話です。
これに近い動きをコードで表すならメソッドAがメソッドBを呼んでいるときに、Bの処理を見終わったころにはAの直前までの処理は何してたっけ？ってなるようなやつです。
記事を見るだけではなく、定期的に思い出そうとすることで、それが短期記憶から長期記憶に保存され、脳に負荷をかけず物事が当たり前のようにできるようになります。
定期的にこの記事を見返すなり、他のリファクタリング関連の記事を見るのが重要です。
次の項目から本題に戻ります。
レビュー前からあったクソコードはどうするのか
コードレビューにより、レビューイはクソコードを書かない、レビュアーはクソコードをはじくようにしました。ですが、レビューを設ける前から存在していたクソコードはだれがメンテするのかという課題が残ります。
結論から書くと、関連機能を修正する人が直します。
画面XのボタンAのコード修正が入ったときに、「Xのコードすべてをリファクタリングする」というわけではなく、ボタンAの既存コードをリファクタリングできそうならするということです。
リファクタリングしようとした結果、デグレする可能性があるので、テストパターンがわかっていて、かつ修正や動作確認に時間がかからないなら対応します。
冗長なコードを排除することだけがリファクタリングということではありません。
変数名をわかりやすくしたり、コメントを残したりすることで見やすくするのもリファクタリングなので、簡単なものから着手していきましょう。
悩み
勉強しない人にはクソコードについて知ってもらえない
日頃からQiitaや参考書を見ない、勉強会に参加しないという人にはクソコードのクソさを共有できないです。
その人がクソコード書いてる張本人なら一番知っていてほしいんですけどね。
こうなるとコードレビューのときぐらいしか、知ってもらえる機会がないです。皆さんがレビュアーやる機会があったら、レビューイにクソコードについて叩き込んでほしいです。
おわりに
「気づいたらクソコード書いてた」ということにならないよう、お互いに気をつけましょう。この記事を定期的に見るのを繰り返して、長期記憶に保存してくれると嬉しいです。
あと、トイレの後は手を洗ってください。
最後まで読んでいただき、ありがとうございます。
関連