2552
1241

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?

More than 3 years have passed since last update.

糞コードは直すな。

Posted at

とりあえず落ち着け。

みなさん、毎日なにかしらのコードを読み、開発する日々を送っていると思います。そんな中で、

糞コードは死ぬべきである!!絶対に直すべき!!

という感情に取りつかれてしまうことがあると思います。自分の技術力に自信のある人ほど、無理やりにでも直そうと試みると思います。それがどんな修羅の道か。そして、糞コード修正がどんな道を歩むのか。この記事では糞コード修正の罠とありがちなストーリーについて書きたいと思います。

ビジネスとしてのプログラムは本質的に糞である

例えば、「携帯電話の利用料金」のプログラムがあります。
 「携帯電話 透明性高め料金値下げを」という記事もあるように世の中の携帯電話の料金プランはかなり複雑です。例えば、auだと「auでんき」といった電気料金とパックされた電話料金プランがあります。また、「auスマートバリュー」といったプランもあり、家のインターネット回線をau光回線にすることで割引が受けられるプランもあります。他には、家族割で複数回線を契約した場合、契約者が学生だった場合の学割(終了済み)・・・正直これがすべてだと思いませんが、このように何かオプションをプラスして料金を安くするプランはかなり色々あります。そして、これらの割引の条件が重複するパターン。割引条件が重複しないパターン。と、プランを無尽蔵に増やしていくことで、複雑怪奇な条件式が生まれていきます。
こういったビジネス上の複雑さはプログラムに反映されます。これは本質的にプログラムでの解決は難しいです。
 では、ビジネスがすっきりとした料金プランならシステムは綺麗になるか?というと、そんなことはありません。例えば、「Dropbox が初期からの有料プラン利用者に感謝、永久無料化」という記事があります。その題名の通り、初期のDropboxユーザーは有料プランが無料になる施策が行われました。私はこういう施策を見るたびに胃が痛くなります。
安直な運用をしてしまうと、「有料プランの購買が〇月〇日以前のユーザーには、永続無料プランのフラグを1にする」といったことをしがちです。このユーザーを仮にロイヤルユーザーと呼びましょう。これらのユーザーは、初期のサービスを支えてくれた人々のために大切にしたい。といった力学が働きます。しかし、2019年にDropboxは今まで無制限だった接続デバイス数が3つに制限されました。さて、ロイヤルユーザーに制限をかけるべきでしょうか?かけないべきでしょうか?こういう議論をすると、**「制限をかけて有料ユーザーを増やし、実績が欲しいやり手の新参企画職」v.s.「ロイヤルユーザーを大切にしたい古参企画」という対立構造が生まれたり、それによって議論は平行線、開発陣としては「時間がないから早く仕様決めてくれ」なのに「実装する仕様自体は魔境」といった、どこをとっても地獄が生まれたりします。**あとヤバいパターンとして、その辺の事情をうやむやにした状態にされて、結論出ないままプロダクトに出た後に問題になってプログラマが問いだたされるとかもあります。
 Dropboxの内部事情は知らないので、以上のようなことがあったかは知りませんが、割と近い話はどこでもあると思います。

安直に永久とか無制限のビジネスとか作るな。続くビジネスとシステムがマジで死ぬぞ。

「携帯電話の利用料金」の議論では、「今の仕様」が複雑なために、起こりうる複雑性です。しかし、実体としてかなり厄介なのは**「過去に行われた施策で、その施策のユーザーが少ないのに対応しないといけないケース」**。しかも、そのユーザーを新規で作る方法が失われており、当時の開発者もいないのに対応しないといけない。でも、今でもお金を払ってもらっているためには無下にできない。みたいな地獄of地獄のケースもあります。このように、過去の安直なキャンペーンレベルのビジネス施策の負債をずっと重りとして引きずるケースもあります。
"技術負債"という言葉は、よく聞くようになりましたが、最近見たツイートの"技術的連帯保証人"は言いえて妙です。

このように本質的なビジネスの複雑さ、一時のキャンペーンだったものが永久に残り続ける仕様が残っており、それが起因して、コードが整理できない。糞コード化しているものもあります。

金を稼がない綺麗なコード

糞コードにいいことがあります。それは何でしょう?

バグが少ない。

いや、そんなことはない。実際にはバグが起こってる。と思うかもしれません。仕様が複雑すぎて書き直す方がバグが出るというケースがあります。前節の話を踏まえて話すと、ビジネスの歴史的な遺恨により、複雑化されたコードを見て、新入プログラマーは、「なに。この糞コード・・・」となり、リファクタリングしたい衝動に駆られます。しかし、ここには、「新入プログラマーはビジネスを簡単にとらえすぎている」という問題があります。確かに、ユーザーが見る料金プランは"簡単にわかる"ように見せています。しかし、前述したとおり、システムはそれがすべてではありません。
しかし、実力のある新入プログラマーは「これは糞コードだ!」と思い、「ビジネスの知識が浅いまま」作ってしまいます。しかも、「根底が腐っている」と思って、完全にリライトしたくなります。そういったコードは実は読みやすく綺麗かもしれません。しかし、経歴が浅いためビジネスの内容を取らえ切れず、仕様のミスを含んでいる可能性が高いです。一方で、糞コードは、今まで幾多の事故(と多大なる犠牲)を経て、修正を重ねられているため、技術者にとって修正難易度は高く糞コードかもしれませんが、ビジネスを安定的に回すという面では実績がある。という辛い現実があります。
残念ながら**「金を稼ぐ実績のある糞コード」 v.s. 「金を稼ぐか分からない実績のない綺麗なコード」だと短期の経済合理性は前者の方が高くなります。**
よく「銀行のシステム改修にCOBOLエンジニア募集してます。」「COBOLwwww」というのを見ますが、こういう事実があるため、ちょっと笑えない。ということもあります。

自分にとってしか綺麗でないコード

よく勉強しており、本当に良いコードを書いていたとしても、それは、ビジネス上(≠技術的)正しくない場合もあります。例えば、「良いコードのプラクティス」(例えば、早期リターンとか)があり、チーム内で、そのプラクティスを広めようとします。しかし、チームの練度が低いと、局部的に、古い書き方を踏襲してしまい、新しい書き方と古い書き方が混在します。そして、レビュアーの練度が低いと、プラクティスを踏襲していないコードを受け入れてしまったり、コード規約を押し付けたい側としても、「じゃあ、どういう風に新しい書き方で書くの?」というところに答えを持つことができず、受け入れざるを得ないパターンもあります。結局、古いコード規約で統一されてる方が読むコスト低いんだし、そっちのほうがいいじゃん?となり、しかも、古いコードを書いてる方が多数派。そして、前述したように新しく作り直しをするということは仕様が複雑であればあるほど、今まで顕在化しなかった新規バグを作りこむ可能性があります。
残念ながら、新しい書き方をする奴、推奨する奴を悪者にし、変化をしない方向にする政治手法がコスト安で外乱を排除し、(なんなら短期の開発速度は上がり、)チーム内の心理的な平穏を持つことができるという地獄が生まれます。

別の例をお話しましょう。明らかに作りがFortranなのにC++で実装されているプログラムとかあります。一時期ちょっと読んでたコードにGROMACSというソフトがありました。これは特殊な用途なので、内容は割愛しますが、その中に、realという型が存在します。これはC++の複素数の意味のrealではなく実数を表しています。
https://github.com/gromacs/gromacs/blob/acbbcc1367fdb43e7404884e79882f01b296598b/src/gromacs/utility/real.h#L139
この分野のソフトは歴史的にFortranで書かれていることが多く、realというのはFortranで実数を表します。そういう人たちが、C++に流れてくると、こういう惨劇が生まれ、分野の人にはわかりやすいが、一般プログラマが死にそうになるプログラムが生まれたりします。といっても、昔はintegerとかcharacterも定義されて(たような気がする)て、あれ?おれC++のコード読んでるんだよな?Fortranじゃないよな?と脳がバグった記憶がありますが、久々に見るとマシになったなーと感じました。
このように**何が綺麗か何が良いコードなのか。何をもって良いとするのか。というのは、そのプログラムを作る人々、行っているビジネスに依存します。**そのため、これが世の中のベストプラクティスだ!と言って、持ってきても、チームの問題を解決できなければ効果はありません。

安易なLinterは死を呼ぶ

最近では、ソースコードにLinterや静的解析ツールを入れるのは普通になってきたと思います。
そういう流れに乗って、今の既存のコードにLinterを入れよう。という技術選択をすることがあります。

Linterを入れると死ぬぞ

嘘だと思うでしょう。Linterでソースコードを整形したらコードが壊れるなんてことないと思うでしょう。残念ながら、私は今までに2,3回あります。なぜか?といわれると分かりません。今まで、そのように壊れてきたコードはテストが書かれておらず、動作保証されていませんでした。そして、一気にソースコードを書き換えるために、複数のコードが原因なのか、単一のコードが原因なのか解明するのも困難なものが多いです。
これは、自分の中の傾向として、リフレクションを使っているものはかなり危険です。例えば、以下のようなスクリプトです。

class Validator:
    @staticmethod
    def validate_user(user):
        print("call validate_user")

    @staticmethod
    def validate_article(article):
        print("call validate_article")


def validate_obj(obj):
    validate_type = obj.__class__.__name__.lower()
    f = getattr(Validator,"validate_"+validate_type)
    return f(obj)

class User:
    pass

class Article:
    pass

user = User()
validate_obj(user)

article = Article()
validate_obj(article)

こういうコードがあると、もう私は逃げ出したくなります。リファクタリングが非常にしにくいです。まず、Userのクラス名を変更したとき、IDEが追ってくれるのは、既定のClass名ぐらいなので、このようにリフレクションでアクセスしようとしたコードが動かなくなります。また、バリデーション自体が1つのクラスにまとまっているため、バリデーションを必要とするクラスと密結合になります。そして、こういうコードほどテストがなかったりするので、何のクラスがどう依存しているのかが判然とせず、下手にリファクタリングできない。というループに陥ります。したがって、Validatorとその周辺のコードが膨張し続けながら腐敗していく様を見とどけるしかない。という動きになります。リフレクションを使うコードは最終手段で、「納期が間に合わない。コードをきれいに書いている時間はない。そこで高度な技術とともに時間短縮をする大技。」という形で局部的に用いられている場合を見ます。しかし、「納期に間に合わせる。」という利益を得られる結果、「Linterがかけられない」という技術負債を被ることになります。個人的には、安定稼働が必要なプロダクトにリフレクションは使ってもらいたくないです。
だからといってLinterをかけない。というのも腐敗を進めるだけなので、基本的には、「ボーイスカウト・ルール」が良いと思います。来た時よりも美しく。ソースを書いたとこから美しく。新しく作ったところに限りLinterで処理するようにソースコードの管理体制を作っていくのが個人的な流儀です。そうすることプロダクトに比較的安全にLintをかけることができます。

どうせやる気はなくなる

あぁ!!このコードむかつく!!これ直しましょう!!

は大体死亡フラグです。糞コードが糞コードたる所以で、大体、1日、2日で直すことができないから糞コードなのです。こういう怒りに任せて、糞コードを若さのパワーで押し切るのは、大体みんな通ってきた道です。そして、日がたつにつれて、目は死んでいき、糞コードに目を向けるのが辛くなる。しかし、啖呵切った手前、直さないわけにはいかなくなり、精神的な重圧が強くなってくる。
一時の感情に任せるな。**必要なのはやる気ではない。啖呵を切る男気でもない。根気と、それをやりきる戦略です。**あと、チームメンバーを敵に回してメリットはないです。

糞コードに敬意を。

残念ですが、あなたの給料は糞コードに支えられている。

上記したようにプログラムにビジネスの遺恨を残している場合もあります。しかし、それはビジネスの試行錯誤の歴史であり、もしかしたら、そのキャンペーンを行わなければ、今のプロダクトが無かったかもしれません。そのため、糞コードを直したいのであれば、そういったプロダクトに関する歴史的事実とも向き合わなければなりません。
これは、ちゃんと表明しておきますが、

糞コードは直すべきです。しかし、安直に直すな。

ということです。そりゃ私だって糞コードは嫌いで、保守しやすい方が良いです。しかし、"とりあえず全体にLinterを入れてみる。"とか、"大規模改修のついでにリライト"とか、"テストとかよくわからんけど、これから自動テストを導入してみよう"とかをやると、かなりの可能性で事故を起こします。そして、若い気力で、"俺がやる!"も大体息切れします。そのため、一旦引いた眼で状況を把握し、技術負債を解消するうえでどういう力学が働くのか、なぜ技術負債が発生してしまったのかを考え、やるべきことを1つずつ積んでいくのが大事だと私は思います。

2552
1241
70

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
2552
1241

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?