今年の四月に新卒入社した会社で 先輩とリファクタリングしていることがあるので、そこでリファクタリングに関して学んだことを連載という形で記事にしていきます。
今回は、long methodのリファクタリングを解説をしていきます。
間違っていることがあれば、指摘いただけると泣いて喜びますmm
使う言語
- scala（たまにjava）
long methodとは
名前の通り、メソッドに含まれるコードの量が多すぎるということです。
なぜcode smellなのか？
そもそもなぜメソッド内のコードの量が多すぎることが問題なのでしょうか？
2つの点が考えられます。
1.単純に読みにくいので処理を追いにくい
例えば、同じメソッドを100行ほどのコードが書かれていると、三行目くらいで読むのを諦めてしまいます。
2. ユニットテスト（UT）がしにくいので、バグの温床になる
同じ関数内で複数の処理をしていると、UTがしにくいので、網羅されていない、ふとした瞬間にバグの温床になります。
以下の例では、createUrlメソッドの中で、redisから値を取得する処理と、実際にUrlを作成する処理の二つが含まれています。
def createUrl(key: String): String = {
val keyword = redisClient.getCaches(key).orElse("hoge")
s"http://localhost:9000?q=${keyword}"
}
long methodになる原因
ではなぜ関数が長くなってしまうのでしょうか？
主に、以下の2つの原因が考えられます。
1. 関数内で複数の処理をしている（複数の概念を持ってしまっている）
2. 関数内の処理の抽象度が揃っていない
ソリューション
上記にあげた2つの原因を１つずつ潰していけば、long methodのcode smellは無くなりそうです。
long methodの解消方法はたくさんあるので、それぞれどの方法を当てはめるかは、ぶち当たってるlong methodよります。
なので、一概にこれ！！！とは言えないですが、
一般的に、long methodの解消法の大きな概念として以下１つのことを行えば解消できると思います。
関数を抽出する
この関数を抽出すると言っても、様々な捉え方があります。
単純に同じクラス内で、privateメソッドに抽出する。
- before
メソッド内の処理が複雑化しており、また具体的な処理がifの中だけで書かれているので抽象度が揃っていないので、読みにくくなっています。
def search(searchResult: SearchResult): String = {
if(searchResult.totalHits > 0 || searchResult.result.nonEmpty) {
Ok(
createResponse(.....)
)
} else {
NotFound
}
}
- after
ifの中身をprivateメソッドに持って行くとメソッド内の抽象度が揃って、読みやすくなりました。
def search(searchResult: SearchResult): String = {
if(existSearchResult(searchResult)) {
Ok(
createResponse(.....)
)
} else {
NotFound
}
}
private def existSearchResult(searchResult: SearchResult): Boolean = {
searchResult.totalHits > 0 || searchResult.result.nonEmpty
}
他のクラスにメソッドを持って行く。
先ほどの例を凝集度の観点でさらにリファクタリングすると。
def search(searchResult: SearchResult): String = {
if(searchResult.hasContent) {
Ok(
createResponse(.....)
)
} else {
NotFound
}
}
case class SearchResult(
totalHits: Long,
result: Option[Result]
) {
def hasContent: Boolean = totalHits > 0 || result.isDefined
}
SearchResultの凝集度が上がって、さらに読みやすくなりました。
凝集度に関しては、次のlarge classリファクタリングの記事で解説します。
複数の処理をしている場合に、それぞれの処理をメソッドに切り分けてあげる。
- before
最初の例では、一つのメソッドの中で複数の処理を行ってしまっています。
def createUrl(key: String): String = {
val keyword = redisClient.getCaches(key).orElse("hoge")
s"http://localhost:9000?q=${keyword}"
}
- after
URL作成処理と、redisからkeywordを取得する処理を完全に分けます。
createUrlメソッドの中で、getKeyWordを呼び出すのではなく、
それぞれのメソッドの呼び出し元（今回の場合はmainメソッド）で、それぞれのメソッドを呼び出します。
そうすることで、処理の抽象度が揃い格段に読みやすくなります。
def main() {
val keyword = getKeyWord(key)
createUrl(keyword)
}
private getKeyWord(key: String): String = {
redisClient.getCaches(key).orElse("hoge")
}
private def createUrl(key: String): String = {
s"http://localhost:9000?q=${keyword}"
}
まとめ
今回は、long methodに関するリファクタリング例を紹介しました。
以下の二点を意識すると、必然的にsmellのないメソッドを定義できるようになると思います。
-
１つのメソッドでは１つの処理をする
-
関数内の処理の抽象度を合わせる。
最後までありがとうございました。
次回は、large classに関するリファクタリング解説を書いていこうかと思います。
では。