はじめに
少し前に、私はソースコードの中の依存関係を解析するツールを開発しました。このツールはC#のソースコード用にはRoslynを用い、C++のソースコード用にはlibclangを用いて抽象構文木を生成しました。これが思ったように動作しているかどうかを検証するために、それから私は未使用メソッドを特定するための機能を実装してみました。この結果、C++のコードのパースよりもC#のほうがずっと正確な結果を出したため、C#のパーサの開発に更に注力し、人々がより洗練されたC# のコードを書けるようにすることにしました。
最初にこのツールは冗長なメソッドが存在する場所の行に目印を付けていましたが、この問題の大きさが明らかになったため、それらの行を自動的に削除するオプションが実装されました。典型的な解析においては、可能な限り冷酷にソースを刈り取るために反復的なツールの実行を行うことになります。その後、ビルドを成功させてとテストをパスするために変更を切り戻すための数回のサイクルを実行します。失敗する理由は、ツールがうまく動作しなかったり、例えばリフレクションやコード上の取り決めといった既知の制限が理由です。
ツールの解析対象として、GitHub上のさまざまなC#のリポジトリに狙いを定めました。プロジェクトの選定にあたっては、私が使用しておりコントリビューションを行いたいと思っているものを基準としました。結局、プルリクエストとは自分のブランチの変更に関する議論をコミュニティに依頼するためのものです。ツールは冷酷で私は初めて関係者とオンラインでやりとりをするため、外交的に振る舞いあまりに多くの人々を攻撃しないのが私の望みでした。コントリビューションを行い、それに続く議論に関わることにより、課題に対する私の理解はずっと進みました。この記事は、もっと広いコミュニティにそのことを広める試みとなります。
解析されたGitHubのプロジェクト
Mono.Cecilは.NETのコードをC#にデコンパイルするためのソフトウェアで、JB Evain氏により書かれました。ツールによりほんの36行を削除することが提案され、レビュー後にJB氏はブランチにマージすることよりも、これと独立していくつかの変更を行うことを選択しました。
Automatic Graph Layoutは公式なMicrosoftのプロジェクトであり、Lev Nachmanson,氏、Sergey Pupyrev氏、Tim Dwyer氏、Ted Hart氏、そしてRoman Prutkin氏により開発されているツールです。このツールはグラフと有向グラフを描画することができ、さまざまなインタラクティブな図を表示するためにVisual Studioで使用されています。プルリクエストは4674行の削除を行うために行われ、それらの多くは(2015年に非推奨とされた)SilverLightとともに動作するためのものでした。このブランチは修正や議論なしにマージされました。
Roslynは.NET Foundationからのチームにより維持されている現代的なC#のコンパイラです。このときのpull requestは18364行を削除するものでした。これに関して素晴らしい議論が行われたことにより、以下に示す分類の大部分に関する知見を得ることができました。一度にマージするには明らかに大き過ぎたため、代わりに多くの個別の課題が作成されました。
MSBuildは、Visual Studioのユーザーなら誰でも親しみのあるべきMicrosoft公式のプロジェクトです。解析により3722行の削除を行うpull requestが作成されましたが、不幸にも提案された変更をレビューする時間がチームにはありませんでした。
解析対象の最後は.NET Core foundational librariesに含まれるSystem.XMLのアセンブリーでした。これらのライブラリーは.NET foundationによって維持されており、チームにはデッドコードを消去するための追跡課題がありました。この課題は、サイズ削減(より知られている単語でいうとデッドコード消去)に関連する各アセンブリープロセスにより対処されました。それからどのコンパイルコードが削除されたかを特定するために、削減前と削減後の差分を生成していました。そしてこの差分により消去されるべきソースコードが判明し、実際の作業はおおむねボランティアコミュニティにより実行されました。
以下の例はたった5つのプロジェクトのものであり、信頼性のある統計が特に欠けているためここからどんな結論を導くのも適切ではありません。
Mono.Cecil | MSAGL | Roslyn | MSBuild | CoreFX (System.XML) | |
削除された行数 | 36 | 4674 | 18364 | 3722 | 427 |
最初のコミット | 2010/04/12 | 2015/02/22 | 2014/03/18 | 2015/03/12 | 2014/11/08 |
著者の数(主要な著者の数) | 39 (1) | 25 (4) | 285 (31) | 90 (6) | 526 (29) |
MSBuildに関しては、長い歴史を考えると最初のコミット日は注目に値します。それぞれのコントリビューションの評価なしに著者の数を数えることはほぼ確実に無意味であるため、目立ったコントリビューターを大まかに記載しました。とは言っても、私は以下のように推測してみました。
- 新しく開発されているプロジェクトは成熟したプロジェクトよりも冗長なコードが多い
- コードはいつか必要になるだろうという仮定の上で記述されている
- テストが少ない場合には発見されるバグも少ない
- チームが大きい場合は冗長なコードも多くなる
- チーム内のコミュニケーションの規模はチーム人数の二乗に比例する。Brooksを参照のこと。
検査結果の分類
このツールは冗長なメソッドを検査しますが、他のどんな検査とも同じように解析が成功したり失敗したりする可能性があります。
真の陰性:
- コードは有用である。
偽陰性:
- テストのためだけのコード
- デッドコード
真の陽性:
- 意図的に放置された開発
- メソッドは追加されたが、変更状況からみてこれらは永遠に使用されない。
- 不完全な変更
- あるメソッドへの参照を全て削除するようにコードが変更されたが、後続のコミットがこのことを考慮していない。これはときどきoxbow codeとも呼ばれる。リファクタリングの際の意図しない成果物であるかも知れない。
- 未使用変数
- 理想的にはコンパイラ警告かIDEにより報告されるだろう。
偽陽性:
- このツールの不備
- 退行
- 開発中に不要であると確認されたメソッドがもう呼び出されないように、このメソッドを変更した時に発生するものである。明らかに、この退行を検出するために検査する実装は存在しない。
- 条件コンパイルの欠如
- 典型的には、これは#if DEBUGガードがされていないものを指す。Roslynはリリース版向けビルドのために使用されていたが、このことが対応するバイナリを膨張させる結果となった。
- 意図せず放置された開発
- これはテストデータの特定の場合ではあるが、対応するテストケースが存在しなかった場合に関連している。一般的な場合では、よりコードが追加されるまでの一時的な問題であると私は考えている。
- このサブシステムにおいては未使用
- このメソッドを使用している箇所を発見するには解析対象のコードが不十分だった場合である。これは、リフレクションによるものと同様になりうるが、公開メソッドを解析した場合に主に発生する問題である。よくある例は、テストのためのヘルパー機能が対応するテストコードでは処理されない場合であり、このことはテストのためのヘルパーのあるべき場所に関する設計上の問題を提起している。
- デバッガー専用のメソッド
- いくつかのプロジェクトにはデバッガーアタッチされた(潜在的にはリリースビルドに対して)ときに状態を出力するメソッドがあった。これらは何らかの方法で解析されないように印をつけるべきである。
ツールの失敗は推論の欠如のために必然的に発生するものではないことを特筆すべきです。これらの場合のいくつかは他のツールにより補完されています。例えばコンパイル警告は問題のいくつかを通知しているべきですし、重複コード検出ツールも有用です。
冗長コードの影響
偽陰性と真の陽性はまとめるとYAGNIの実例であると考えることができ、以下の議論を提起します。
- 膨れあがったソースコード
- 私自身が過去にサイズの大きなファイルを開くことができないテキストエディタに遭遇したことがある。
- 開発者の理解力は、脳内に同時に保持しておける独立した実体の数により制限を受ける。通常これは7つとされているため、メソッドを削除することは推論能力を改善することに役立つ。
- ソースを読んで理解するために必要な開発者の時間はおそらく大部分が不必要である。
- 現代的なIDEはソースから抽象構文木を作成するため、冗長コードには処理時間が多く必要である。
- 膨れあがった実行ファイル
- これは特に携帯端末において問題であり、アップグレードの際にはストレージを確保するためにアプリケーションを消去することを迫られる。最終的には、機能性があまりに妥協したものであれば、端末自体をアップグレードする必要に迫られる。
- 膨れあがったランタイム
- ハッカーに対するアタックサーフェスの増大はセキュリティの懸念を引き起こす。
- 組み込み装置のいくつかのアプリケーションは最初から全ての動的メモリを確保するため、重複コードは使用可能なヒープを減らすことにつながる。
- パフォーマンスの低下
- デッドコードの特定の場合には、実行時にプロセッサーサイクルを浪費する。
- 信頼性の低下
- デッドコードの特定の場合には、実行時にクラッシュする可能性がある。
- 保守性の低下
- コードが予期せず冗長でなくなった場合に振る舞いが予測できない。例えばKnight Capitalは44億ドルを失った。
- コードカバレッジの割合は偽陰性により増加し、偽陽性により低下する可能性がある。
- 不要なテストはテストスイートの実行を遅くする。
- 静的解析ツールの処理対象が増加し、処理時間が増える。
- ソフトウェアの設計やアーキテクチャが実際よりも複雑に見えてしまう。
重複コードのために生じうる幅広い問題を考慮に入れると、重複コードはコードの臭いとして扱うのが一番良いでしょう。
既存の冗長コードの管理
冗長コードに対処する方法には4つあります。
- 無視する
- 関連する維持コストはあるとしても、最低でもまだコンパイルは可能であろう。
- 推奨されない。
- 実行ファイルから自動的に除去する
- コンパイル言語の場合、リンカに存在する何らかのデッドコードの除去オプションを使用することで除去できる。
- 動的言語の場合、除去のために実行時にtree shakingを使用すること。
- これもまた推奨されない。
- 追い出す
- コメントアウトするか、プリコンパイラ命令を使用する。
- IDE内のコードもしくはコメントの折りたたみ機能を使用し、見えないようにする。
- 削除する
- バージョン管理システムを活用し、探して見つけられるように参照用の古いコピーを保持しておく。
- あるいは、あなたは二度とそれを必要としないかも知れない。
ソースコードが変更されるときは、そのコードが本当に冗長であることを確実にするために、どんな変更でもそのコードの部分に関して知識のある誰かのレビューを受けるべきです。
新規開発の管理
目的が開発中に新しい重複コードを持ち込まないということだったとして、どんな戦略を実行すべきだろうか?もし部分的なコミットを許可すると、コードベースに対して一時的に冗長なコードが追加されることになりそうである。このことは機能ブランチの導入と、テストがカバレッジを提供し、ソースの静的解析に問題がない場合にのみマスターブランチにマージすることにより緩和できる。機能ブランチのもう1つの利点は、開発が放置されたときにマージをしないままにするということができる点である。
著者について
Zebedee Mason氏はCAD/CAM/CAE業界で25年の経験を持ち、近年SLAM業界に移動した数値アナリストである。何年にも渡り、彼はレガシーのコードベース(いくつかは彼が子供の頃に作成されたものである)に対して作業を行い、数多くの研究レベルのコードを商用ソフトウェアのコンポーネントに書き換え、いくつかオリジナルのアルゴリズム(商業的秘密保持により承認されていない)を記載することもあった。このような背景により彼にはソフトウェアツールに対する評価眼が備わっており、前任の開発者やIT管理者による興味深い選択にも関わらず、生産性を向上させるための数々のツールを制作した。