code smell リファクタリング[long method編]

リファクタリング

記念すべき１つ目は、long methodのリファクタリングを解説をしていきます。

 

long methodとは

名前の通り、メソッドに含まれるコードの量が多すぎるということです。

 

なぜcode smellなのか？

そもそもなぜメソッド内のコードの量が多すぎることが問題なのでしょうか？

2つの点が考えれると思います。

 

  • 単純に読みにくいので処理を追いにくい


例えば、同じメソッドを100行ほどのコードが書かれていると、三行目くらいで読むのを諦めてしまいます。
 


同じ関数内で複数の処理をしていると、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つの原因が考えられます。

 

  • 関数内で複数の処理をしている（複数の概念を持ってしまっている）
  • 関数内の処理の抽象度が揃っていない

ソリューション

上記にあげた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に関するリファクタリング解説を書いていこうかと思います。

 

では。

参考