1375
1440

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 5 years have passed since last update.

あきらめるにはまだ早い!ソースコードの品質向上に効果的なアプローチ

Last updated at Posted at 2014-07-18

エンジニア組織を強くするための本を出版しました

Qiitaでエンジニアリングをめぐる様々なコミュニケーションの問題とその解決策や考え方を書いてきた。それらの背後にあるエッセンスをこの度書籍として出版するに至りました。

エンジニアリング組織論への招待
~不確実性に向き合う思考と組織のリファクタリング

この書籍は、エンジニアリングを「不確実性を削減する」という第一原理で捉え直し、様々なエンジニアリングとその間のコミュニケーションをめぐる現象を説明していくものです。

はじめに

この記事は数百万行の動的型付き言語のWebアプリケーションのリファクタリング、アプリケーションアーキテクチャの再設計の経験を基に、有効だと思われる考え方やアプローチを抜粋して紹介するものです。言うまでもなくあらゆるコードベース、アーキテクチャにおいて有効なものとは限りませんので、各々の環境や状況から適切に判断してください。

コーディングガイドラインを作ろう

まず、何はともあれ「暗黙知」によるコード評価は避けましょう。コーディングガイドラインを作成し、それに準拠することが重要であるというコンセンサスをとりましょう。

また、各言語のスタイルなどは既に存在しているスタイルガイドを踏襲することで、新たに加わったメンバーでも学習コストを下げることができます。

たとえば、こういうところから見つけてみてはどうでしょう。
http://blog.verygoodtown.com/2013/03/development-user-style-guide/

このスタイルガイドに関しては、自動検知、自動修正ができるようなものが望ましいです。

  • Ruby:rubocop
  • Perl:perltidy
  • Go:gofmt
  • JavaScript:jshint,jsfmt

のようなフォーマッタや、lintツールを利用し、スタイルの修正や発見といったことに時間を費やさずにすむようにしましょう。

コーディングガイドライン自体にはスタイルの他にもフレームワークの規約や後述するようなレイヤリングアーキテクチャなどの依存関係のルールなど多岐にわたってきます。

これらも自動的な検知やテストを行うことで、発見できるようにしましょう。

コードレビューをしよう

コードレビューを行うことは重要です。コードレビューのためのツール、プロセスを導入しましょう。

コードレビューの利点は様々ありますが、個人的に最大の利点は「コードについて話し合う文化ができる」ことでしょうか。

コードレビューにおいては、自動的に検知できるものに関しては適宜自動化していくのがよいでしょう。設計上の問題について、話し合い、適切な粒度でレビューを行うことで、コミットログもきれいになりますし、教育的効果も望めます。

アジャイルプロセスを運用しよう

プロセスの種類は問いませんが、チームでアジャイルプロセスの運用をはじめましょう。この際、設計に関してチームという単位で共有できるように心がけましょう。

アジャイルプロセスのソースコード品質に対するメリットは主に3つです。

  • 練度の高いメンバーから低いメンバーへの教育効果
  • ドメイン言語の共有による実装品質の向上
  • 要求と設計の不一致の早期発見

逆にこれが成し遂げられないアジャイルプロセスは形骸化した意味のないものになりがちです。
メリットとなるポイントが実現できているか、定期的なレトロスペクティブを行い改善される状況を作りましょう。

自動テスト、継続的インテグレーションを導入しよう

もし、あなたのプロジェクトに継続的インテグレーションの仕組みがないのであれば、直ちに導入するべきです。
ある程度テストが増えてから、ではなく、なるべく早い方がいいです。鶏が先か卵が先かの議論になりがちですが、鶏を生み出してしまえば、卵は生まれます。

テストがすべてGreenでない限り、Deployをしないと制約づけることで、自動テストには一定の「強制力」が発生します。

この際、テストカバレッジをどの程度要求するのかについてもコントロールできるようにしておくといいでしょう。ただ、先を急いでテストカバレッジの値を高く要求するとテストにかかる工数が膨大になってしまい、かえって開発速度を低下させてしまいます。

後述する「レイヤードアーキテクチャ」の採用とともに、各レイヤーごとに適切なカバレッジC0,C1の値を決めていくなどすることで、費用対効果を調整しながら自動テストの導入を図ることができます。

http://ja.wikipedia.org/wiki/継続的インテグレーション
http://www.techmatrix.co.jp/quality/validation/coverage.html

静的解析のメトリクスを測定し、自動テストに組み込もう

静的解析とは、ソースコードを動作させずに字句的に解析し、品質を測定する手法です。各種言語で様々な指標を得るためのライブラリが存在するでしょう。

最近のCIツールは、ファイルごとのメトリクスなどを表示することもできます。
数値として表示することも可視化のためには重要ですが、その運用には自動テストと併せて利用するのがいいでしょう。

静的解析による指標の導入方法の例としては、たとえば:

  • 「メソッドの長さ」を50lines以下に制約したい
  • コーディングガイドラインでは推奨されていたが、実際には守られていない箇所が多数ありそう。
  • すべてのコードに対して、この解析を行ったところ、30カ所のガイドライン違反が見つかった

こういったケースの場合、

  • ガイドライン違反を検知するテストを自動の対象として追加する
  • すでにガイドライン違反をしている30個のメソッドを列挙し、テスト対象のblacklistとして分離する。
  • 新規ファイルを対象にpre-commit時に検査できるようにする
  • それぞれの修正についてissue、ticket化する
  • blacklistの追加を禁止する

というようなステップを踏んでいくと良いでしょう。

静的解析の対象としては、

  • 命名規則
  • 循環的複雑度
  • 1クラス、1ファイルの長さ
  • 依存関係の制約(安定依存の原則、循環依存禁止の原則)

など、適切なもの、測定可能なものを選び取り入れましょう。
あと忘れがちなのはリポジトリ中の設定を書きこんだyamlやjsonファイル、テンプレートファイルが規約に則った形式であるか、malformatでないかなども重要なテスト対象です。

表明を使おう

表明とは、アサーションのことです。ソースコード中に満たすべき条件を組み込むことで、自動テストや静的解析でカバーしきれない問題などにも対処することができます。デバッグ時環境や本番環境など、条件に応じた適切なアサーションを組み込めるようなライブラリを導入しましょう。
http://ja.wikipedia.org/wiki/%E8%A1%A8%E6%98%8E

Debugログによる表明

現代的なWAFを利用しているのであれば、ログ情報のレベル管理はなされていることでしょう。Debug環境時にのみ出力されるログを利用した開発者へのアラートは、ある程度有効な選択です。

特に、ランタイム時にしか検知できない表明が動的言語によるアプリケーションの場合多く存在します。静的な型を持った言語であっても、テンプレートエンジンが動的な型を許容する場合があり、注意が必要です。

また、Debug時のログでしかないため、本番環境に与える影響もないので、安心して追加することができます。

しかし、しばしばDebug時ログによる制約は無視されます。安心して追加することができるため、強気にログの追加をしがちですし、多くなればなるほどそれを当たり前のものとして受け入れてしまう傾向があります。

潔癖な人が直して回るという状態が出来上がるか、全員が無視するという末路も考えられます。

Debug時パニックによる表明

Debugログよりも、ハードに攻めたい人にはお勧めの制約として、Debug時パニックによる制約があります。
Debug時のみ、表明違反をした場合に例外を発生させ、システムを止めるというものです。

開発者は、これについて修正しなければ開発を進められないため、制約する力が強くなります。

動的な自動検知全般に言えることですが、「検知はできても、統治はできない」たぐいのものです。
コード中からリストアップして見つけたり、それが起こりうる場所を探索したりと言ったことができません。

たとえば、ライブラリの入力値アサーションなど、すでにあるものではなく、これからおこるかもしれないところで適応するのがいいでしょう。

本番(一部)サーバーログによる表明

自動テストと比べ、表明を利用したコードの品質管理の場合、すべて表明違反を列挙し管理することが難しいとお話ししました。なぜなら、動かして初めてわかるケースが多いからです。完全な列挙とまでは行かないでしょうが、たいていの場合うまく行く方法としては、本番環境でログ出力をし、それを集計してレポートすることです。

数が膨大になるのであれば、一部サーバのみ適応するか、確率的にログ出力をするなどして数の問題をクリアしましょう。

アプリケーションサーバー以外の箇所もアプリケーションコードが適応される領域(たとえばcron scriptやjob queue worker)などがあるのであれば、そこも対象に入れておく方が安全でしょう。

列挙したものは、テスト対象のブラックリストにしたり、チケット化するなどして管理下におきましょう。
たとえば、一元的な置換ができないようなDeprecated/ObsoleteなAPIなどを管理したい場合にはこの方法で統一して管理すると良いでしょう。

コードチャーンを分析し、メトリクスに取り入れよう

コードチャーン(Code Churn)とは、平たく言えば、git logやgit blameの結果です。
http://blogs.msdn.com/b/askburton/archive/2004/09/09/227515.aspx

静的解析のメトリクスよりも潜在的なバグの発見に効果的であろうということで、近年注目を浴びています。
Googleがbugspotsとして、OSS化しています。
http://google-engtools.blogspot.jp/2011/12/bug-prediction-at-google.html
https://github.com/igrigorik/bugspots

Microsoft Researchの大規模な研究結果を基にしています。bugspotsは非常に簡単なrubyプログラムです。
http://research.microsoft.com/apps/pubs/default.aspx?id=69126

わかりやすくいえば、複数人で複数回にわたって編集されたファイルは複数の目的でコードが編集されている訳ですから、「単一責務原則」を違反している可能性が高く、潜在的にバグを内在していそうだということです。

bugspotsは、最近コミットされたコードが高く危険視されるアルゴリズムであるため、バグが出尽くしていないコードのなかでさらに危険な部分を見つけることができます。

一方、古いコード全体で、固定的に危険なモジュールを見つけるのであれば、単純にコミット人数を数えたり、修正数を数えたりするだけでも危険なモジュールを探し出すことができます。

自動検知のターンアラウンドを早くしよう

自動テストで検知できるようなガイドライン違反であれば、より早く開発者に知らせることで開発のターンアラウンドが早くなります。1つはPre-Commit時にテストscriptを走らせる方法、もう1つはエディタのプラグインやfswatchコマンドとして「ファイル保存時」などに走らせる方法です。

Pre-Commit scriptによる制約

新たな追加コードに限定して、制約を加えたいのであれば、もっとも単純な方法はコミット時検査です。
開発者でコミット時検査scriptを共有し、コミットタイミングで制約違反となるコードを検出します。

エディタプラグインによる制約

静的解析で検証できるもののうち、高速に処理できるものはEditorプラグインとして開発するのもいいでしょう。これは開発のライフサイクルの中で、間違いを見つけるイテレーションが早くなりますので、効果が高いです。ただし、たいていのWeb企業では、エディタの自由を認めていますし、強制するのは難しいでしょう。

独立したscriptとして作成し、出力フォーマットを決めておけば大抵のエディタでプラグイン化するのは簡単になりますので、そういったscriptとエディタプラグインを追加するリポジトリやルールなどを決めておくことで、知識の共有がスムーズにはかられます。

また、fswatchなどのファイルシステムのwatcherにガイドライン違反検出やテストのscriptなどを走らせるのも悪くないでしょう。

レイヤリングアーキテクチャを組もう

レイヤリングアーキテクチャとは、アプリケーション全体を階層的に分割し、その依存関係について規約を持つアーキテクチャのことです。たとえば、ドメイン駆動設計中で記述されているレイヤリングアーキテクチャは次のようになります。

  • プレゼンテーション層:見た目やユーザーインターフェースに関連するレイヤ
  • アプリケーション層:ビジネスロジックを除く、アプリケーションとしてのコーディネーションを行うレイヤ
  • ドメイン層: ドメインに関する知識を担当するレイヤ。上位レイヤから依存される。
  • インフラ層 : DB操作やその他技術的な解決を行うレイヤ、各レイヤから依存される。

基本的に、上から下方向への依存は許可され、その逆は許可されません。
各モジュールやパッケージがどの階層に属しているかを決め、禁止されている依存関係を持たないように静的解析テストなどで検知するようにします。

開発開始時点からレイヤリングが意識されているのであれば、問題はないのですが、あとから追加する場合には静的解析と同様にブラックリストを管理するなどの段階的な適応をしましょう。

依存関係の解析による指標としては、
「安定依存の法則」を満たすためのメトリクスとして、「Instability(不安定性)」があります。

Instability (I = Ce / (Ca + Ce))

このような指標を使いながら、「悪い依存」を見つけ出し、継続的に監視するようにすれば
障害の影響範囲を局所化したり、予想だにしないバグの発生を防ぐことができます。

PubSubで依存性を分離しよう

PubSubパターンとは、

出版-購読型モデル(しゅっぱん-こうどくがたモデル、英: Publish/subscribe)は、非同期メッセージングパラダイムの一種であり、メッセージの送信者(出版側)が特定の受信者(購読側)を想定せずにメッセージを送るようプログラムされたものである。出版されたメッセージにはクラス分けされ、購読者に関する知識を持たない。購読側は興味のあるクラスを指定しておき、そのクラスに属するメッセージだけを受け取り、出版者についての知識を持たない。出版側と購読側の結合度が低いため、スケーラビリティがよく、動的なネットワーク構成に対応可能である。
http://ja.wikipedia.org/wiki/%E5%87%BA%E7%89%88-%E8%B3%BC%E8%AA%AD%E5%9E%8B%E3%83%A2%E3%83%87%E3%83%AB

というような、アーキテクチャパターンの1種です。
ある程度アプリケーションが成長し、各ドメイン間で相互作用を持たなくてはならない場合に有効です。

たとえば、Qiitaを例にとると記事の投稿に際して、

  • 投稿内容のサマリーをタイムライン用のDBに書き込む
  • 投稿のレコメンドシステムに最新投稿を教える
  • 検索エンジンに投稿内容をインデクシングする

といった複数の関心領域にまたがる処理が走るとします。(勝手な想定です。)
これを同一のメソッドの中など書き込むと各ドメインは深く結合し、見通しがわるくなります。

こういったケースのときに、
記事を投稿したというイベントを定義し、それをレコメンドシステムや、検索エンジンなどのシステムで監視させます。

イベントの発火を受けて、各システムが自分たちに必要な処理を行うように設計すると、
記事を投稿するためのモジュールの追うべき責務は、「イベントを発火すること」として単純化され、各システムとの依存を疎結合にすることができます。

実際にQiitaは外部サービス向けのAPIとしてWebhookを提供していますので、Qiita本体とは完全に疎な形で、
特定メンバーの投稿の通知botなどを作成することもできます。

サービス、パッケージの分離を行おう

リポジトリ分割制約

依存関係のレイヤリングに強制性をつけたいのであれば、リポジトリの分割は有効な手段です。
たとえば、「インフラ層」から「アプリケーション層」への依存を追加したくないのであれば、「インフラ層」を別のリポジトリで管理し、「アプリケーション層」からはgitのsubmodule機能やパッケージマネージャ経由で利用するようにすれば、依存関係がないことを示すことができます。

この際、「インフラ層」のモジュールの結合テストか少なくともビルドができるかどうかのテストがないと、完全には依存関係を切っていることを保証できない点に注意が必要です。

また、特定の依存関係を禁止したい、たとえば「実装継承」による依存などを禁止したい場合はリポジトリ分割だけではだめで、静的解析などによる「インフラ層」から「アプリケーション層」「ドメイン層」「プレゼンテーション層」への実装継承検知する必要があります。

システム分離制約

システム分離制約は、データベースおよびアプリケーションサーバを分離し、相互のやり取りをRPCやメッセージキュー、特定ファイルの交換などに分離することによって、アーキテクチャを分離する方法です。

たとえば、検索機能を実装する際に、検索エンジンと表示画面を分離するときなどがわかりやすい例です。
他にもサービス間の分離のために認証機能のみのAPIサーバーと認証後の機能を分離するなどがいい例でしょう。

この制約はサービス間の連携が本当に粗結合であるときに有効です。
連携が密結合な場合、内部的なAPIリクエストの数が爆発したり、障害点が増えたりとレスポンスの悪化を招いたり、障害検知が複雑化しがちです。

サービス間の結合性の見通しが立ってから行うのがいいでしょう。

たとえば、外からサービスだけ見て考えるに、
はてなブックマークとはてなブログ程度の結合であれば分離したほうメリットが大きいでしょう。
はてなスターとはてなブックマークの場合、悩ましいところ。ブックマーク一覧ですべてのブックマークに対して、スター数を取得しなければソーティングを実装することがむずかしい。一定期間のキャッシュや2重管理のDB管理をしてパフォーマンスを実現するなど、トレードオフを判断する必要がありそうです。

APIで分離した場合、それを利用するクライアントコードを都度開発するのが面倒になりがちです。できればサービスディスカバリやインタフェースの共有によるライブラリコード生成などのサポートがあるとなおいいです。

また、どのアプリケーションがどのサービスを利用しているのかを検知、管理する仕組みも必要な場合があります。あるAPIの変更がどのようなクライアントから利用されているかが見つけづらくなるため、クライアント情報を登録管理したり、APIバージョニングのルールと仕組みを決めると行ったことも必要になってきます。

定期的なインスペクションを行いガイドラインを見直そう

一口にソースコード品質と言っても様々な観点があります。

  • 理解可能性(Understandability)
  • 完全性(Completeness)
  • 簡潔性(Conciseness)
  • 移植性(Portability)
  • 一貫性(Consistency)
  • 保守性(Maintainability)
  • 試験性(Testability)
  • ユーザビリティ(Usability)
  • 信頼性(Robustness)
  • スケーラビリティ(Scalability)
  • 構造化の度合い(Structuredness)
  • 効率性(Efficiency)
  • セキュリティ(Security)

そのソフトウェアのステージや、経済状況などに応じて「何について」「どの程度」対策を行うのかという指針が
アーキテクチャ設計の重要な指針になります。

これらの指針は、常に陳腐化する可能性があります。ビジネス要件やシステム全体に対する定期的なインスペクションを行い、指針を決めていくのがアーキテクトの仕事となります。

専任のアーキテクトがいない場合は、少数のチームによって行い、最終意思決定者を置くべきでしょう。
あまり多くなってしまっても「船頭多くして船山に上る」と言ったことになりかねません。

ただし、ガイドライン修正時にはそれを遵守するというコンセンサスは必ずとっておきましょう。

できるところからはじめよう

多くのWebアプリケーションは、当初の想定とは異なる方向に進化しながら、その柔軟性を欠いていきます。その結果、アプリケーション開発はだんだんと遅くなります。少数のチームで始まったものであっても、だんだんと人数が増え、すべてを把握しているエンジニアは退職したり、別のことをはじめるためにかり出されることもままあります。

また、99.9%以上のアベイラビリティを要求されるため、大規模なメンテナンスも難しくなります。
ユーザー数のスケーラビリティと開発者数や機能数に対するスケーラビリティも同時に対応しなければなりません。

そのためにはそのタイミングごとに適切なアーキテクチャ設計に変化、進化していかなければなりません。
初期設計と異なり、稼働中のアプリケーションに対するアーキテクチャ変更は一筋縄では行きません。そのため、理想と現実のギャップが常に存在しており、その一歩が踏み込めなくなることもあるでしょう。

稼働中のWebアプリケーションにおいて、突然ガイドライン上の制約を増やすことはとても難しいです。とくにそれが暗黙的に守られている状況の場合、十中八九守られていない箇所が存在します。

問題は、「守らないプログラマがいる」ことではなく、「守らずにコミットできること」「守っていない箇所を検知できないこと」「守っていない箇所をコントロールできていないこと」にあります。

稼働中のすべてを修正するのではなく、コントロール下に置くために段階的な制約を定義し、順次適応していくことで、意識することができ、徐々に修正されていく状況を作ることができます。

参考文献

新人プログラマに正月休み中を使って読んでみてほしい技術書をセレクトしてみた。

1375
1440
2

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
1375
1440

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?