こんにちは。コミュニケーションアプリ「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)
}