LINEヤフー Tech Blog

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

コード品質向上のテクニック:第60回 出る杭を「ひっぱたく」な

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

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

出る杭を「ひっぱたく」な

以下の compressFile 関数は、その名の通り、指定されたファイルを圧縮する関数です。

fun compressFile(...) {
    try {
        val workingDirectory = createDirectory(...)
        val fileToCompress = file.copyTo(workingDirectory)
        CompressionUtil.compress(fileToCompress)
    } catch (exception: IOException) {
        val logger = LoggerService.get()
        val logMessage = ...
        val params = ...
        logger.log(Log.Error, logMessage, params)
    }
}

今は 10 行程度の関数ですが、さらにコードが増えると、プライベート関数に分割してリファクタリングしたくなると思います。そこである開発者が、SLAP (Single Level of Abstraction Principle) というプログラミング原則に従って、この関数を分割しようと考えました。SLAP は、スコープ内 (関数など) のコードの抽象度を揃えるというプログラミング原則です。以下のコードでは、compressFile 内のコードを「圧縮のメイン処理」と「エラー処理」に分けることで、抽象度の一貫性を維持しつつ compressFile の単純化を試みた結果です。

fun compressFile(...) {
    try {
        compressFileInternalOrThrow(...)
    } catch (exception: IOException) {
        logError(...)
    }
}

// May throw IOException
private fun compressFileInternalOrThrow(...) {
    val workingDirectory = createDirectory(...)
    val fileToCompress = file.copyTo(workingDirectory)
    CompressionUtil.compress(fileToCompress)
}

private fun logError(...) {
    val logger = LoggerService.get()
    val logMessage = ...
    val params = ...
    logger.log(LogLevel.Error, logMessage, params)
}

このリファクタリングに改善の余地はあるでしょうか?

メインのロジックをひっぱたきすぎない

このリファクタリングでは、メインのロジックの可読性 の観点で悪化してしまっています。リファクタリング前の compressFile では、「作業用ディレクトリを用意する」・「ファイルをコピーする」・「圧縮する」という動作の流れがわかるようになっていました。しかし、リファクタリングをした結果、compressFile の中身を読んでも「圧縮は失敗するかもしれない」ということしか分からなくなってしまっています。関数の中身を読みたいということは、シグネチャやドキュメンテーションからは読み取れない動作を知りたいはずなのですが、そのためには compressFileInternalOrThrow を読まなければなりません。(さらにつけ加えると、compressFileInternalOrThrow がどのような例外をスローするかについて、呼び出し元が気をつける必要が新たに発生してしまっています。)

場合によっては SLAP を意識しすぎず、重要なコードは具体的なまま維持してしまっても良いでしょう。言い換えると、メインのロジックの抽象度をサブのロジックよりも下げる ほうが良いこともあります。以下の例では、圧縮するというメインのロジックは関数に切り出さず、エラー処理だけの抽象度を上げています。

fun compressFile(...) {
    try {
        val workingDirectory = createDirectory(...)
        val fileToCompress = file.copyTo(workingDirectory)
        CompressionUtil.compress(fileToCompress)
    } catch (exception: IOException) {
        logError(...)
    }
}

private fun logError(...) {
    val logger = LoggerService.get()
    val logMessage = ...
    val params = ...
    logger.log(LogLevel.Error, logMessage, params)
}

「サブ」に該当するロジックの例としては、以下のものが挙げられます。

  • エッジケースの処理
  • エラー処理
  • セットアップと後処理
  • リソース管理
  • 戻り値や別の関数呼び出しのための型変換

このようなサブのロジックが長々と書かれていると、メインのロジックの理解を阻害することがあります。エッジケースやエラーケースなどを理解するためには、そもそもその関数が何をしているかを知る必要があるため、まずはメインのロジックを理解できるような構造にするというのは合理的です。

SLAP は、「抽象度において一貫性を保つ」原則と言い換えることができます。コードの一貫性を保つべきという主張は、この SLAP 以外にも数多くあります。しかし、コードの一貫性の維持は、可読性や変更容易性といったコードの品質を向上させるための手段 であることに気をつけなければなりません。無理に一貫性を保とうとして、かえって分かりにくい構造にしたり、技術的負債を増やしたりしないように注意しましょう。

一言まとめ

メインのロジックの抽象度は、サブのロジックよりも下げた方がよい場合もある。

キーワード: abstraction level, function, extraction

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

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