長期プロダクトの負債脱却で意識するコードの片付け

はじめに

この記事は、DMMグループ Advent Calendar 2024の11日目の記事です。

デジタル作品を販売するオンラインECサービスでバックエンドエンジニアをしておりますyachiyと申します。
上記のサービスは10年以上の長期で運用されているサービスであり、直近はサービスの一部プロダクトについて、負債脱却観点でリプレースを実施しています。
この記事では、私が現在実施している負債脱却の案件の中で遭遇した、大きな障害とならないまでも時間を浪費したり、将来時間を浪費してしまうことになり得る課題について紹介いたします。
もし読者様のプロダクトで同様に当てはまるものがありましたら、年末ということもありコードの大掃除も検討してみてはいかがでしょうか。

嘘つきのお片付け

嘘つきコメント

長期で運用されているプロダクトには様々な罠が潜んでいます。
その中でも直近の案件で特に多く、経験上もっとも時間を無駄にしまったのがコード上の嘘つきコメントです。

皆様はコメントをどのような場合に記載しますでしょうか。
大きく分けると以下の2つのパターンが多いと思います。

  • どのような実装をしたのかの説明
  • なぜその実装をしたのかの説明

この2つの内、後者は実装の背景であり、事実として変わることは少ないですが、前者は注意が必要であり、しばしば嘘をつくことがあります。

なぜ嘘をつくようになるのか

コメントを記載した当初は可読性などを意識して善意で記載されているものが大半であり、嘘をつくつもりは全く無いと思います。
しかし、プロダクトが長期で運用されていると多くの改修が発生し、コードのみを変更し、コメントを修正し忘れるという状況が発生してきます。
プロダクトの成長に直接影響が大きいのは動作するコードの部分であり、改修の際はその実装やテストに注力していくため、コメントの修正はどうしても二の次になり、抜け漏れが発生しがちになってしまいます。
改修の際にコードだけでなく、コメントも同様にメンテナンスされていることが徹底されていれば問題は発生しにくいのですが、レビューの体制が整っていなかったり、工数の兼ね合い等様々な要因からコメントの変更漏れを完璧に防ぐのは難しくなってきます。
この、メンテナンスされなかった処理内容を説明しているコードが実際のコードの内容とは乖離していくことによって、コメントが嘘をつくようになります。

嘘をつかれることによる実害と対策

リプレースの案件では特に既存仕様の調査をしばしば行います。
調査の過程で処理内容の説明であるコメントがあると、そのコメントを信用し調査を進めるのですが、実装内容とコメントが乖離していると、後々コメントの内容と異なる挙動になっていることに気が付きます。
テスト等で早めにコメントに嘘があったことに気付ければ調査をし直し、その分の時間の無駄ですみますが、嘘に気づかず案件が進行していった場合は障害となってしまうケースもあります。
善意で書いたコメントがビジネスの損害につながってしまっては元も子もありません。
コメントを書く際は処理内容の説明は記載せず、実装している背景などコードからは読み取れない情報を記載するようにし、既存の実装の説明で可能なものは片付けることを検討してみてはいかがでしょうか。
実装内容の説明については、以下のようなアプローチで代替を検討しましょう。

  • コード自体から振る舞いを読み取れるようなアプローチ
  • メソッドを分割し、メソッド名で処理内容を簡潔に表現するアプローチ

後者については改修時にメソッド名が変更されなければ、コメント同様嘘をついてしまうのですが、それでも処理から呼び出される動くコードの一部ではあるため、コメントよりはメンテナンスされやすいという次善のアプローチになります。

嘘をつくドキュメント

コメントだけではなく、ドキュメントも嘘をつくことがあります。
特に長いプロダクトであれば、ドキュメントは実装当初のスナップショットであり、メンテナンスはされていないことがしばしばあります。
現状と乖離している可能性も十分考えられるため、あくまで参考程度に考えると良いかもしれません。
ドキュメントも常にメンテナンスするアプローチも考えられますが、どうしてもコードと二重管理であり、長期でプロダクトを運用する中で増え続けた、多様な機能に関するドキュメントをメンテナンスし続けるには工数を始めリソースの限界があります。
せっかくなら、実際に動作しているプロダクションコードからより仕様を読み取りやすくなるような工夫を出来ないか検討しましょう。

不要なコードのお片付け

デッドコード

長期で運用されているプロダクトには、その処理を通過しないのにも関わらず記載されているコードが存在していることもあります。
私が良く見るパターンでは時限的な施策として実装され、対象の期間が終了したものの削除されていないコードです。
このようなコードは無駄に複雑度や調査コストを上げ、実装されている意味もなく誰も幸せになりません。
見つけ次第、片付けることを検討しましょう。

無意味なローカル変数とelse

調査で処理を読む過程で、無意味な記載がある場合も塵が積もって山となり、メンテナンスコストを増大させます。
以下のような簡易的なじゃんけんの判定を記載した処理があるとします。

<?php
public function rockPaperScissors($hand): string
{
    $result = '';
    if ($hand === 'rock') {
        $result = 'win'
    } elseif ($hand === 'scissors') {
        $result = 'draw';
    } else {
        $result = 'lose';
    }
    return $result;
}

このとき、 $result は以下のように条件ごとに即時の戻り値の確定が可能であるため、意味のない不要なローカル変数となります。
判定後即時の戻り値の確定が可能なためelseも不要となります。
この無意味なローカル変数とelseがないだけでもコードがより簡潔になり、調査時間の短縮につながるので同様の箇所があれば片付けることを検討しましょう。

<?php
public function rockPaperScissors($hand): string
{
    if ($hand === 'rock') {
        return 'win'
    } 
    if ($hand === 'scissors') {
        return 'draw';
    } 
    return 'lose';
}

参考文献

『現場で役立つシステム設計の原則 〜変更を楽で安全にするオブジェクト指向の実践技法』

最後に

現場によっては当たり前の細かいことの紹介でしたが、長期で運用されているプロダクトの場合は、塵が積もって山となり、仕様などの調査に差し支えるケースがしばしばあります。
皆様のプロダクトにも細かい塵が溜まってきていないか調査や片付けを実施し、新年気持ちを新たにスタートダッシュを決める準備をしてみてはいかがでしょうか。

宣伝

長期で運用されているデジタル作品を販売するオンラインECサービスを共に成長させていくエンジニアを募集しております。 ご興味があれば覗いてみていただければと思います。

dmm-corp.com