maybe daily dev notes

私の開発日誌

完璧より簡潔が良い - バグ修正編

最近タイトルの考えに至ることが短期間に2度あったので、改めて書き出してみる。

問題提起

例えば今、何らかのコードのバグ修正を試みている状況だとする。バグの原因は考慮漏れのエッジケースがあったとしよう。 修正のアプローチとして以下2つがあるとしたら、どちらを選ぶかという話。

  1. 完璧: そもそもエッジケースが生じない実装にまるっと書き換える
  2. 簡潔: 考慮されていなかったエッジケースに対する分岐を新たに追加し、バグを回避する

完璧・簡潔というのは私がこの記事で勝手に付けた呼び方なので、他所では通じないことに注意。

結局答えは「場合による」なのだが、この記事ではもう少し解像度を上げることを目指す。

具体例

上の2択だと抽象的すぎるので、一つ具体的なバグで考える。この記事を書くに至ったきっかけの一つ。*1

github.com

これはAWS CDKのバグなのだが、要はある条件の論理和を取る関数があり、その関数に渡す条件の配列の長さが 10n+1 (11, 21など) のときにエラーが起きるというもの。 *2

下にイメージを示す。 Orの引数の配列は上限長が10という制約があるため入力の配列は長さ10ごとに分割されるのだが、元々の実装で「Orの引数の配列は下限長が2」 という制約が見逃されていたためにバグが発生した。

generateOr([1,2,3,4,5,6,7,8,9,10,11]) # 入力
↓
Or([1,2,3,4,5,6,7,8,9,10,11])
# Orの引数の配列は上限長が10という制約があるので、長さ10ごとに分割する
↓
Or([Or([1,2,3,4,5,6,7,8,9,10), Or([11])])
# Orの引数の配列は下限長が2という制約があるので、Or([11])はエラーになる

これに対する修正方針はいくつか考えられるが、以下に代表例を2つ挙げる:

  1. 完璧: 1個余る場合は、その配列だけOrを適用しないようにする
    • Or([Or([1,2,3,4,5,6,7,8,9,10), 11)])
    • 根本原因に対処するアプローチ。その分実装の変更量は増える
  2. 簡潔: 入力の長さが 10n+1 のときは長さ9ごとに分割する
    • Or([Or([1,2,3,4,5,6,7,8,9]), Or([10, 11])])
    • 既存の処理を使いまわしながら少ない実装変更で済むアプローチ

上記の抽象的な話と対比されたい。今回は、こうした選択肢がある場合にどちらが良いかという話をする。

完璧より簡潔が良いこともある

私自身は、これまで基本的には1の完璧アプローチを好んできた。

2の簡潔アプローチは対症療法のようなもので、根本的な解決とならない場合もある (実際上の具体例では、配列長が90n+1の場合に同じ問題が再発する。) また、バグを含むコードというのは不必要に複雑なためにエッジケースが生まれていることもあり、そのような場合は根本から書き直したくなりがちだった。

直感的にも、1のアプローチをより優れたものだと考える人は多いのではないだろうか。

しかしながら、改めて考えると簡潔アプローチにも良い点はある。

簡潔アプローチの良さ

簡潔アプローチの良さとして、下記が思いつく。

正しさ・破壊的変更のないことを確認しやすい

簡潔アプローチではしばしば既存の処理に分岐が追加されるだけなので、基本的には以下を確認すれば実装の正しさが分かる:

  • 分岐の条件は必要十分か
  • 分岐内での処理は正しいか

もう少し一般化できるかもしれないが、上記の具体例で考えればひとまず納得はできるだろう。 少なくとも分岐の条件が適切であれば、破壊的変更が生じることはない。

対して完璧アプローチでコードを大幅に変更すると、従来うまく動いていた部分にもバグが発生する可能性を確認する必要がある。 不意に破壊的変更が生じている可能性も無数に考えられる。結果として、確認範囲は大きく広がる。

簡潔アプローチは修正に問題がないことを確認するのが容易である。

レビューしやすい

上記の理由の副産物として、レビュワーも差分を承認しやすくなるだろう。

レビュワーの心理の一つとして、自分の承認したPRでクリティカルなバグが発生することは防ぎたいものだ。 完璧アプローチは差分の影響範囲が膨大で、バグが発生するかどうか判断するのに多大な労力が必要になることもあるだろう。

特にOSSでは、見ず知らずの開発者からパッチが飛んでくることもある。 面識のない相手であれば、せめて変更を確認しやすいパッチがより好まれるのも不自然ではない。

必要性の薄いコードの追加を避けられる

一般に、システムは機能が多ければ多いほど良いというものではない。 機能が増えるほど、メンテナンスすべきコードの量も増えるためである。 このため、必要性の薄い・不明な機能は必要性が判明するまで実装するのを避けるというのも、一つの合理的な選択と言える。

これを念頭に完璧アプローチを考えると、報告されたバグ修正の「ついでに」他のエッジケースへの対応が含まれることも多い。 これが直ちに悪いこととは言い難く、むしろ気が利いていると判断されることも多いだろう。 一方、誰も文句を言っていないケースに対する修正によりメンテすべきコードを増やしているので、コスパが悪いとも言えるかもしれない。

どちらが好まれるかは状況によるだろうが、一概にバグをすべて修正するのが最適だとは限らないことに注意されたい。


私の考えた限りでは、簡潔アプローチにも上記の利点がある。 個人的な直感では完璧アプローチのほうが優れているような気がしていたが、状況次第では簡潔アプローチを採るのが良いことは普通にあると思われる。

そういえば前職では共通基盤的なサービスを運用しており、ユーザーからは安定稼働することが強く求められた。 当時は自分も保守的なアプローチとして、コード差分が少なくなるような、簡潔アプローチに近い選択をすることが多かったように思う。

人によってはこんなの当たり前だろ?と思うかもしれない。 私自身は最近仕事で新規開発が多く安定性は二の次だったので、別の観点から物事を見直す良いきっかけになった。

コードの正しさを証明するのが難しい件

そもそも完璧アプローチを採ったとして、それをマージさせるのは大変だという話もついでに。

そのようなパッチをマージさせるには、次の方法がある:

1. コードの正しさを証明する

変更後のコードが正しく動くことを客観的に示し、レビュワーを納得させる。これは理想的だが、現実的には以下の通り難しい。

単に「既存の単体テストが全てパスしています」というのではしばしば不十分である。 元々実装されているテストがすべてのケースをカバーしているとは限らず、「単体テストのパス」=「コードが正しい」とは判断できないため。

では単体テストを充実させれば良いのかというと、いずれにせよ無限にある入力パターンをすべてテストすることはできない。 境界値テストなどの手法もあるが、これも一つのヒューリスティクスにすぎないので、それ以外にバグがある可能性は完全には否定できない。 こう考えると、そもそもが悪魔の証明に近いのかもしれない。

この問題を解消するために、形式手法と呼ばれる、コードの正しさを論理的に証明する方法がある。 私は詳しくはないが、これはこれですべての問題をうまく形式化できるのかとか、形式手法のコードを実際の実装と一致するようにメンテし続けられるのかなどの問題は素人考えでも思い当たる。

形式手法の限界については、以下の記事がとても良くまとまっているように見える。上記に思いを馳せた上で以下の記事を読むと、なるほど納得できることが多い。

qiita.com

あるいはハイブリッドな手法として、形式手法的な考え方をうまく図や自然言語で説明できれば、正しさについてより説得力をもたせられる気がする。 ここはまだ自分の中では感覚的なものにとどまっており、あまり言語化できてない。要精進。

2. 問題が報告されたら直ちにロールバックする覚悟でマージする

コードが正しいことを完全に証明するのはコストが高いのは分かったので、ある程度で妥協して、あとは実地試験すれば良いという考え方。 問題があった場合の計画を事前に立てておき、例えばロールバックなどの対応をする。

昨今運用上のベストプラクティスとしてエラーバジェットを定義しましょうとよく言われるが、その目的の一つがこうしたリスクを取れることだろう。 コードの正しさを100%示すのは非常に大変だが、80%で良ければ2割のコストで確認できるかもしれない (パレートの法則的な)。

また、カナリアリリースやBlue / Greenデプロイもそのための方法と言える。誤ったコードをデプロイする際のリスクを低減する手段であるため。 上の具体例に出したAWS CDKのようなライブラリだとまた話は違うが、これも新バージョンのリリース後すぐにアップデートするユーザーは少ないので、ある種そのパイオニアたちがカナリアになっているとも見ることができるだろう。

パッチの作成者自身がオーナーシップを持っているサービス・コードなら、この方法はよく採用されているように思う。 とはいえOSSでは実際に対応するのはメンテナなので、非メンテナがこの方法を提案しても受け入れられ難いかもしれない。

3. 正しいコードを書く人間だという権威を得る

これも現実的にはよくある話だと思う。

ある程度実績を積むことで、「その人のパッチは多くの場合正しい」という権威を得られる場合がある。 これにより、明確に正しさを証明していないコードについても、マージはされやすくなるかもしれない。

これは直感的には非合理 (コードの正しさはコードだけで判断すべき) では?と思ってしまうが、実際はどうか。 確かにコードの正しさを完全に証明する必要がある場合はその通りで、先入観を捨てた確認が不可欠である。

しかし上にも書いたように、実際のところ完全な証明というのはそもそも難しい。 多くの場合、コードの正しい「可能性が高い」ことを確認するのが現実的な妥協点である。 レビューするパッチの作者が実績のある開発者であるときに、正しい可能性が高いと早めに判断して省力化するのは、統計的にも合理的な考えだろう。


なお「絶対に落とせないシステム」というのも世の中はあるようで、その場合2,3の方策は見向きもされないかもしれない。 私も以前人工衛星というミッションクリティカルなシステムの開発に関わっていたので想像はつくが、あらゆる手を尽くして設計・実装の正しさを証明することになるだろう (レビュー、シミュレーション、信頼性工学の手法、品質試験、etc...)。 とはいえコードの正しさを証明することも前述の通り困難であるし、未知の欠陥がある前提で、fail-safe・fault-tolerantな設計にも全力を注ぐのだが。 文字通りの「絶対に落とせないシステム」というのは要件定義を誤っている可能性があり、むしろ落ちても大丈夫なシステムを目指すのがベターではないだろうか。

話は逸れたが、今回タイトルの件について考察を深めることができた。 上の例に挙げたAWS CDKへのパッチが、どちらの方針でマージされるのかにも注目である。

これってバグ修正特有の問題?

バグ修正編と名打ってみたのは、今回は特に既存機能を変更する時に絞った観点で考えたかったため。

一般論として、新機能にバグがあるよりも、既存機能の今まで正常に動いていた部分にバグを増やしてしまう方が深刻度は高い。 前者はユーザーがそもそも使えない機能として使用を回避するだけで済むが、後者は既に使っているユーザーがただちに不利益を被るためである。

このため人は新機能を書くときよりも、バグ修正をするときこそより慎重になり、完璧より簡潔を目指す方針に価値が生まれる。

ただし、似たような話は新機能開発時にも成立する。初めから完璧を目指すと、結果的に様々な不利益が生まれることがある (premature optimizationなど)。これは今回の話とは異なる観点のため、また別の記事でまとめたい。

まとめ

特にまとまらないが、この辺りで締めることにする。3行まとめ:

  • 既存コードの修正においては、必ずしもすべてのバグを完璧に・美しく修正するのが正義ではない
  • 見えているバグのみを愚直に対応するのも合理的な選択である
  • 一つの方針のみを考えて満足しないよう注意したい

最後に、今月のもなちゃんをパチリ。ピジョンという名前なのか、配色が似ているせいか、ベビーカーをいたく気に入ったようだ。

*1:きっかけのもう一つはこのPR。これは元々(私目線では)完璧アプローチのパッチを提出し、結局簡潔アプローチに近い方法で書き直してマージされた。実際のところ私自身元のパッチがあらゆるケースで動くのか自信を持てなかったので、妥当だとは思う。めちゃくちゃ既存コード消せてたんだけどね。

*2:実際はAWS CDKはCloudFormationテンプレートを生成するソリューションなので、CloudFormationの論理和を取る関数 conditionOr を生成するためのCDKの関数ということだが、本筋ではないので簡略化している。