Skip to main contentSkip to footer
Search
LINEヤフー Tech Blog

LINEヤフー株式会社のサービスを支える、技術・開発文化を発信しています。

コード品質向上のテクニック：第69回 コード品質のための My tips

こんにちは。コミュニケーションアプリ「LINE」のモバイルクライアントを開発している石川です。

この記事は、"Weekly Report" 共有の連載の第 69 回です。LINEヤフー社内には、高い開発生産性を維持するための Review Committee という活動があります。ここで集まった知見を、Weekly Report と称して社内に共有しており、その一部を本ブログ上でも公開しています。（Weekly Report の詳細については、過去の記事一覧を参照してください）

コード品質のための My tips

今回のレポートはいつもとは異なり、コード品質の改善で私が心掛けていることを 5 点取り上げます。

1. 読み" "書きの隙間にご注意ください

コードを書く際は、思いついた内容をそのままコードにするのではなく、他の人がそのコードをどう理解するのかを意識して整理する ことが必要です。アイディアが思い浮かぶまでの思考過程をコードに書き出しても、読みやすいコードになるとは限りません。これは、コードを書くときと読むときとで思考過程が異なるのが理由でしょう。コードを書くときはボトムアップに考えることが多いと思いますが、逆に、読むときはトップダウンに理解しようとすることが多いのではないでしょうか。

「foo と bar に値を代入する」というコードを例に考えてみましょう。foo と bar に代入する値は、条件 isX によって変わるとします。このコードを実装するときは、次のような思考過程を経るかもしれません。

  1. isX がtrueの場合、foo = 42 で bar = "WOW!"
  2. isX がfalseの場合、foo = 24 で bar = "MOM?"

これをそのままコードに書き出すと、以下のようになります。

if (isX) {
    foo = 42
    bar = "WOW!"
} else {
    foo = 24
    bar = "MOM?"
}

このコードを読むとき、最初に目につきやすいのは if による条件分岐の部分でしょう。しかし、if を見ただけでは、isX で処理が異なるということしか分かりません。このコードが何をするのかを理解するためには、まず、isX が true の場合と false の場合それぞれで、foo と bar に値を代入しているということを把握する必要があります。そして、その代入は true と false で共通することだという理解を経て、ようやく「これは foo と bar に値を代入するコードだ」ということが分かります。単純なコードの割に、全体を理解するまでのステップが多くなってしまっていると言えます。

このコードが何をしているのかを短時間で理解できるようにするためには、コードを読む側として最初に知るべきこと、つまり「このコードは foo と bar に値を代入する」ことを目立たせる必要があります。これを達成するには、以下のように条件分岐を代入の内側に閉じ込めると良いでしょう。

foo = if (isX) 42 else 24
bar = if (isX) "WOW!" else "MOM?"

こうすれば、コードの左側を斜め読みするだけで、「これは foo と bar に値を代入するコードで、その値は条件 isX によって決まる」ということがすぐに分かるようになります。

この例に限らず、コード全体を理解しやすくするためには、状態の変化や関数のメインの目的をネストやコールスタックの浅い方に置くことで際立たせ、エッジケースの処理のような補助的なロジックを隠したり抽象化したりすると良いでしょう。特に、外部の状態を変更するコードや大域脱出するコードがネストの深い部分に埋もれていると、そのコードが何をしているのかを理解するのが難しくなります。

コードを書き終えた後は、「初めてこのコードを見た人は、どのような思考過程で理解するのか？」を自問し、「コードを知らない人」の思考を自分の頭の中でエミュレートする と良いでしょう。

この書き手と読み手の思考の違いは、コードだけでなく、ドキュメンテーションやレビューコメントにも応用 可能です。レビューコメントを書くときの思考過程は、まずコードの問題点を発見し、なぜそれが問題であるのかという理由を整理し、改善案を考えるという手順を踏むことが少なくありません。この流れをそのままレビューコメントにすると、問題の理由が先に説明され、その後にどうすべきかのアクションが提示されることになります。しかしこの書き方では、コメントを読む立場として、「結局どうしてほしいのか」が分からないままに長々と問題点とその理由を読まされることになります。レビューコメントを書くときは、先に「どうすべきなのか」を提示して、その理由や参考情報を後述するという形式のほうが好ましいでしょう。

コード、ドキュメント、レビューコメントのどれであっても、書くときと読むときで思考過程が異なるということに意識するだけで、より円滑に開発することができるようになります。

関連するレポート

2. バグらぬ先の検査

コードを書くときは、そのコードを使う人が「正しい使い方」を理解できるかどうかを意識してください。加えて、可能であれば 最初から誤った使い方ができないように設計する とより好ましいです。何らかの理由で誤用の可能性を避けられない場合は、そのコードを単一の関数/クラス/モジュールに閉じ込める と良いでしょう。

ここで注意するべきこととして、「正しい動作」や「誤用されにくいコード」にも、以下のように、いくつかの段階がある点が挙げられます。

  • コンパイラやリンターなどによって静的に検証される
  • テストコードですべてのケースを網羅されている
  • 実行時アサーションによって検出できる
  • ドキュメントによって定義されている
  • 暗黙のルールで決まっている

このリストで上にあるものほど早い段階で、確実に検証できるため、可能であれば上の段階を満たすのが望ましいです。もちろん、いくつかのアプローチを組み合わせることも有効です。特にドキュメントは、コードの変更に追従するコストを支払えるならば、可能な限り他の方法と組み合わせることをお勧めします。

例として、2 つの整数を引数に取る関数 sendFoo を使って説明します。これらの引数は、両方とも 42 以上でなければならないとします。この制約をドキュメントもなしに定義してしまうと、sendFoo(0, 0) のように誤用される可能性があるでしょう。

fun sendFoo(bar: Int, baz: Int) { ... }

これを改善するための案として、ドキュメントで制約を明確にしたり、実行時に値をチェックすることが考えられます。

/**
 * Sends FOO integers as ...
 *
 * Parameters [bar] and [baz] must be greater than or equal to 42.
 * Otherwise, [IllegalArgumentException] will be thrown.
 */
fun sendFoo(bar: Int, baz: Int) {
    require(bar >= 42) { "bar must be >= 42, but $bar" }
    require(baz >= 42) { "baz must be >= 42, but $baz" }

    ...
}

しかしこのアプローチでは、ドキュメントが見落とされるリスクがあります。そして、実際に sendFoo(0, 0) というコードが混入しても、実際に実行されない限り間違いに気づけないかもしれません。今回は引数の制約についてなので、「すべての呼び出し元が正しい引数を渡している」こともユニットテストでも保証しにくいでしょう。

より良い方法の一つとして、型を使って制約を明示的にすることが考えられます。例えば、次のような Foo クラスを定義することで、bar と baz が 42 以上であることを保証できます。of で返された Foo のインスタンスを使うためには、必ず null の取り扱いが必要になるため、呼び出し元に制約を意識させることができるでしょう。

class Foo private constructor(val bar: Int, val baz: Int) {
    companion object {
        /** 
         * Creates a [Foo] instance with the given [bar] and [baz].
         * This returns null if either [bar] or [baz] is less than 42.
         */
        fun of(bar: Int, baz: Int): Foo? =
            if (bar >= 42 && baz >= 42) Foo(bar, baz) else null
    }
}

fun sendFoo(foo: Foo) { ... }

しかし、この方法では制約を作るたびに新しい型を定義する必要があり、場合によってはオーバーキルになることもあります。型をあまり増やしたくない場合の選択肢としては、関数の戻り値としてエラーの状態を返し、その呼び出し元に処理を強制する方法です。(@CheckResult は、Android Kotlin において戻り値の確認を強制するアノテーションです。)

/**
 * Sends FOO integers as ..., then returns true if success.
 *
 * Parameters [bar] and [baz] must be greater than or equal to 42.
 * Otherwise, this function does nothing with returning false.
 */
@CheckResult
fun sendFoo(bar: Int, baz: Int): Boolean {
    if (bar < 42 || baz < 42) {
        return false
    }

    ...
    return true
}

動的型付けの言語で、型ヒントなども使えない状況では、型を使った制約は難しいかもしれません。その場合でも、リンターや静的解析ツールをうまく使うことで、誤用を防げる可能性があります。また、静的な検証ができない場合でも、ユニットテストなどを駆使し、できる限り早い段階で誤用を検出できるようにすることが重要です。

この考え方は、コードを使う場面だけでなく、機能拡張やリファクタリングのために コードを変更する場面にも応用できます。自分自身も含め、開発者が将来誤った方法で変更を行う可能性を意識することで、より堅牢な設計を実現できます。

関連するレポート

3. 原則を疑う原則

世の中には、コード品質を改善するための原則やベストプラクティスが数多くあります。しかし、全ての状況に普遍的に適用できるわけではありません。それぞれの原則やベストプラクティスの背後には、適用できる条件や制約が隠れている ことに気をつけるべきです。これらの条件や制約を無視し、普遍的なものとして取り扱ってしまうと、かえって複雑で分かりにくいコードになる可能性があります。

原則やベストプラクティスを適用するときは、なぜそれが必要なのかを考え、今の状況に合うのはどれか、優先順位を決める 必要があります。

例として、「可視性は出来る限り狭くするべき」という原則について考えます。queryFollowers という関数があり、それが別の関数 fetchUserModel からしか呼ばれないとしましょう。可視性をできる限り狭めるという原則に従うのであれば、以下のように queryFollowers は fetchUserModel のローカル関数 (関数内の関数) として定義されるべきです。

fun fetchUserModel(userId: UserId) : UserModel {
    fun queryFollowers(): Set<FollowerId> {
        ...
    }

    val userName = queryUserName(userId)
    ...
    val followers = queryFollowers()
    return UserModel(userId, userName, ..., followers)
}

しかし、このコードで fetchUserModel が何をしているかを俯瞰したいとき、queryFollowers の定義が fetchUserModel に入っていることで全体像がわかりにくくなる可能性があります。特に、fetchUserModel 内に他の多数の query* 関数が定義されていると、この問題は顕著になるでしょう。

可視性を狭くすることが重要なのは事実ですが、責任を分離し、コードを俯瞰できるようにすることの方が優先されるべきときもあります。今回のコードでは、queryFollowers をローカル関数とせず、別の private 関数として定義したほうが良いかもしれません。private ならば可視性はある程度制限されており、かつ、fetchUserModel が何をしているかが俯瞰しやすくなります。

fun fetchUserModel(userId: UserId) : UserModel {
    val userName = queryUserName(userId)
    ...
    val followers = queryFollowers()
    return UserModel(userId, userName, ..., followers)
}

private fun queryFollowers(): Set<FollowerId> {
    ...
}

どの要素のどの程度優先するべきかは、そのコードの目的や文脈に依存します。素晴らしいと思える原則やベストプラクティスを学んだときは、「素晴らしい」で思考を止めるのではなく、それが適用できない状況や反例についても考察しましょう。そうすることで、コンセプトに振り回される事態を避けられます。

関連するレポート

4. Range(0, 1) に活を求める

一般に、ソフトウェアを開発するリソースは限られています。場合によっては、機能を実装する前にリファクタリングをしたくても時間が足りず、既存の技術的負債を抱えたまま開発することが求められることもあります。そのようなときは、理想的なことができないとしても、可能な範囲での改善を行う ことが重要です。根本的なリファクタリングができない場合でも、以下のような小さな改善を積み重ねる余力があるかもしれません。

  • 現状を説明するコメントやテストを書く
  • 理想的にはコードがどうあるべきかを説明するドキュメントを作成し、それを TODO コメントやイシューチケットから参照する
  • 少なくとも新しいコードでは改善された設計を使用し、それを「リファレンス実装」として残す

特に、「今は A という設計だが、理想的には B にしたい」というアイディアを、他の開発者から見えるようにしておくことは非常に重要です。将来の設計がどうなるかの共有ができていないと、暫定のコードがどうあるべきかも決めにくいです。また、理想形を共有することで、他の開発者がコードを変更する際に、その理想形を考慮した実装にしてもらえるかもしれません。

5. 一に議論、二に議論...

設計、コード、ドキュメントのいずれにおいても、完成後に議論を始めるのではなく、作成しながら議論する のが好ましいです。議論を後回しにすると、根本的なミスなどによる大規模な手直しのリスクがあり、場合によっては双方が精神的に疲弊してしまう可能性もあります。小さな議論を積み重ねる、問題が発覚したときにすぐに対処でき、より安心してタスクを進められます。

そのためにも、議論を楽しむ雰囲気があることが重要でしょう。(コードレビュー中でなければ、) 些細なことや細かいことをあえて議論するのも良いかもしれません。インデントにタブとスペースのどちらを使うべきかといった、ある種「どうでもよい」ことをあえて議論してみたり、自分の本来の意見とは逆の立場で話し合ったりしても良いでしょう。このような議論に慣れると、以下のような効果が得られます。

  • 考えを言語化し、相手に伝える能力が向上する
  • 意見をパーソナリティや感情から分離し、建設的な議論をする方法が身につく
  • 他人の自分のアイディアを組み合わせ、より良い結論を導く経験を得る
  • 議論そのものに対する心理的な障壁を下げる

議論だけではなく、考えの整理や質問でも、気軽にチームメンバーへ話しかけられる雰囲気であれば理想的です。 ただし、議論を自分のチーム内だけに限定してしまうと、知識がサイロ化し、スキルセットが狭くなる可能性があります。他のチームのメンバーにも、積極的に話しかけられれば理想的です。

Thank you

実は、私がレポートを公開するのは今回が最後です。今後は、他の Review Committee のメンバーが、月 1 回程度を目安に不定期にレポートを公開する予定です。引き続き「コード品質向上のテクニック」をよろしくお願いします。

キーワード: summary, tips

コード品質向上のテクニックの他の記事を読む

コード品質向上のテクニックの記事一覧