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

リファクタリング

今回は、long parameter listのリファクタリングをしていきます。

long parameter listとは

その名の通り、引数が多すぎるcode smellです。

引数が多すぎると、可読性が格段に下がります。

引数は最大でも３つまでにしておくべきと、clean codeでボブおじさんは言っています。

主に３つのソリューションがあります。

それぞれ解説していきます。

1. オブジェクトそのものの受け渡し

名前の通り、値ではなくオブジェクトそのものを引数に渡します。

package study.refactoring.longParameterList


class Main {
  def main(args: Array[String]): Unit = {
    val plan = HeatingPlan(10, 20)
    val daysTemp = DaysRoomTemp(2)
    val room = Room(plan)

    val bottom = room.heatingPlan.bottom
    val top = room.heatingPlan.top

    if(daysTemp.withRange(bottom, top)) {
      println("室温が設定値を超えました。")
    }
  }
}

case class Room(heatingPlan: HeatingPlan) {
}

case class HeatingPlan(bottom: Int, top: Int)


case class DaysRoomTemp(temperature: Int) {
  def withRange(bottom: Int, top: Int): Boolean = {
     this.temperature < heatingPlan.bottom &&  this.temperature > heatingPlan.top
  }
}

daysTemp.withRange(bottom, top) で、引数に複数のものを渡してしまっています。

そもそもbottomとtopは、HeatingPlanオブジェクトの値なので、バラバラで渡すのではなく、HeatingPlanオブジェクト自信を引数で渡して、 関数の中で、バラバラにするのが良さそうです。

     ........

    if(!daysTemp.withRange(room.heatingPlan)) {
      println("室温が設定値を超えました。")
    }
  }
}

case class DaysRoomTemp(temperature: Int) {
  def withRange(heatingPlan: HeatingPlan): Boolean = {
    this.temperature < heatingPlan.bottom &&  this.temperature > heatingPlan.top
  }
}

上記のようにすることで、余分な引数の数が減りました。

また、オブジェクトを引数に渡しているので、可読性が格段に上がることがわかります。

現在の室温と、設定した室温を比べているのだなと関数の中の処理を読まなくても一目瞭然になっています。

2. パラメータオブジェクトの導入

値オブジェクトを作ることで、引数をオブジェクトにまとめることができるソリューションです。

package study.refactoring.longParameterList

class Main2 {

  def main(args: Array[String]): Unit = {
    val house = new House()
     val heatingPlan = HeatingPlan(10, 20)

    if(exceedSettingHeatingPlan(house, 10, 20)) {
      println("過去の室温で設定外になっていると場合がありました。")
    }
  }

  def exceedSettingHeatingPlan(station: House, bottom: Int, top: Int): Boolean = {
    station.getHistoryTempList.exists(temp => {
      temp < bottom || temp > top
    })
  }
}


class House() {
  def getHistoryTempList: List[Int] = List(5, 6, 7, 8)
}

10 20をそのまま引数に渡しているので、これはオブジェクトにできそうです。 HeatingPlanオブジェクトを作ります。

// 10 20オブジェクトに格納する
if(exceedSettingHeatingPlan(house, HeatingPlan(10, 20))) {
  println("過去の室温で設定外になっていると場合がありました。")
}

case class HeatingPlan(bottom: Int, top: Int)

exceedSettingHeatingPlanメソッドの引数受け取りの値も、HeatingPlanに変更します。

def exceedSettingHeatingPlan(station: House, heatingPlan: HeatingPlan): Boolean = {
    station.getHistoryTempList.exists(temp => {
      temp < heatingPlan.bottom || temp > heatingPlan.top
    })
  }

これで、オブジェクトを引数に持たすことができました。 可読性がだいぶ上がりましたね。

しかし、これだけだと１つ目のリファクタリングと大した違いはありません。

ここからが、パラメータオブジェクトを作成することの真価です。

作成したオブジェクトに振る舞い（関数）を持たせることができることで、パラメータオブジェクトはただのデータオブジェクトではなく、

振る舞いを持った立派なオブジェクトへと進化することができます。

def exceedSettingHeatingPlan(station: House, heatingPlan: HeatingPlan): Boolean = {
    station.getHistoryTempList.exists(temp => {

      // HeatingPlanクラスに、containsメソッドを生やしています。
      heatingPlan.contains(temp)

    })
  }


case class HeatingPlan(bottom: Int, top: Int) {
  def contains(temp: Int): Boolean = {
    temp < bottom || temp > top
  }

}

凝集度が上がり、betterな設計になってきました。

凝集度の点で、もう一箇所リファクタリングしていきます。

if(house.exceedSettingHeatingPlan(HeatingPlan(10, 20))) {
      println("過去の室温で設定外になっていると場合がありました。")
    }


class House {

  def exceedSettingHeatingPlan(heatingPlan: HeatingPlan): Boolean = {
    getHistoryTempList.exists(temp => {
      heatingPlan.contains(temp)
    })
  }

  private def getHistoryTempList: List[Int] = List(5, 6, 7, 8)
}

exceedSettingHeatingPlanメソッドは、Houseクラスの責務だと思うので、HouseクラスにexceedSettingHeatingPlanメソッドを生やします。

すると、Houseクラスの凝集度は上がり、さらに exceedSettingHeatingPlanメソッドの引数も減りました。

積極的にオブジェクトを作っていくことは、いいことづくしですね。

3. フラグパラメータの削除

フラグパラメータを削除します。

フラグパラメータは、関数が何をするのかを理解するためのプロセスが煩雑になってしまい、非常に読みにくいです。

特に以下の例のように、true falseだけを引数に渡していても、それが何を表しているのかを、関数の中にまで入って読まないといけなくなるので、

非常に罪深いです。

package study.refactoring.longParameterList


class Main3 {

  def main(args: Array[String]): Unit = {
    val order = Order()
    deliverDate(order, true)
  }

  def deliverDate(order: Order, isRush: Boolean): Unit = {
    if(isRush) {
      ???
    } else {
      ???
    }
  }
}

class Main4 {
  
  def main(args: Array[String]): Unit = {
    val order = Order()
    deliverDate(order, false)
  }
  def deliverDate(order: Any, bool: Boolean): Unit = ???
  
}


case class Order()

deliverDateメソッドを見ただけで、trueが何を表しているのか理解できません。

このようなフラグ引数がある場合は、true falseでそれぞれ新しい関数を作ってあげ、それぞれに適した関数名をつけてあげることで、

可読性が上がります。

package study.refactoring.longParameterList


class Main3 {

  def main(args: Array[String]): Unit = {
    val order = Order()
    rushDeliverDate(order)
  }

  def rushDeliverDate(order: Order): Unit = {
    ???
  }
}


class Main4 {
  
  def main(args: Array[String]): Unit = {
    val order = Order()
    BasicDeliverDate(order)
  }
  def BasicDeliverDate(order: Order): Unit = ???
  
}

case class Order()

これでフラグ引数はなくなり、可読性が上がりました。

まとめ

積極的にオブジェクトを作成してあげることが、１つのキーになるかなと思います。

最後までありがとうございました。

次は、feature enzyについて書いていきたいと思います。

参考