Imperative vs Functional by example [closed]

Question

As it currently stands, this question is not a good fit for our Q&A format. We expect answers to be supported by facts, references, or expertise, but this question will likely solicit debate, arguments, polling, or extended discussion. If you feel that this question can be improved and possibly reopened, visit the help center for guidance.

Closed 9 years ago.

I have a method implemented in imperative and functional (I did my best here) ways. The method iterates over ArrayBuffer[Creature], calculates distance to each creature and return the closest or None (if no creatures in the world except 'this').

Imperative:

private def closestEnemy: Option[Creature] = {
  var closest: Option[Creature] = None
  var distanceToClosest = Int.MaxValue

  for(creature <- game.creatures if creature != this) {
    val distance = distanceTo(creature)

    if(distance < distanceToClosest) {
      closest = Some(creature)
      distanceToClosest = distance
    }
  }

  closest
}

Functional:

private def closestEnemy: Option[Creature] =
  game.creatures filter { _ != this } map { creature => (creature, distanceTo(creature)) } match {
    case creaturesWithDistance if creaturesWithDistance.isEmpty => None
    case creaturesWithDistance => Some(creaturesWithDistance minBy { _._2 } _1)
  }

The functional code looks less obvious (probably it can be simplified but I don't see how) and I'm not sure if I am able to read it on fly in a month. My question is it a matter of habit or functional isn't good for this particular case? Did you have such doubts when started Scala? Did your functional skills greatly improved after some time and totaly beat the imperative approach? Please post your experience.

Thanks!

Answer 1

If your primary goal is readability, the following gives the same output as your code:

private def closestEnemy =
  game.creatures.filterNot(_ == this).sortBy(distanceTo).headOption

This seems extremely clear to me—there's very little syntactic noise, particularly compared to your imperative version.

Unfortunately sortBy is more expensive than minBy, and even more unfortunately there's no "safe" minBy in the Scala Collections API (where "safe" means that it returns an Option that's empty when called on an empty list). The following isn't too terrible, though:

private def closestEnemy = game.creatures.filterNot(_ == this) match {
  case creatures if creatures.nonEmpty => Some(creatures.minBy(distanceTo))
  case _ => None
}

So in this case you've run into a legitimate shortcoming of the Collections API (no safeMinBy), but I'd still personally much prefer to maintain this code than your imperative version.

---

As a footnote: it's worth noting that you can use the pimp-my-library pattern to "fix" the Collections API. Just put the following in scope somewhere:

implicit def makeSeqSafer[A](xs: Seq[A]) = new {
  def safeMinBy[B: Ordering](f: A => B) =
    if (xs.isEmpty) None else Some(xs.minBy(f))
}

Now you've got a safe, efficient minBy:

scala> List(1, 2, 3).safeMinBy(_ * 2)
res0: Option[Int] = Some(1)

scala> List.empty[Int].safeMinBy(_ * 2)
res1: Option[Int] = None

And in your case:

private def closestEnemy =
  game.creatures.filterNot(_ == this).safeMinBy(distanceTo)

Answer 2

You can do this simpler by using collect:

game.creatures collect { case creature if creature != this => (creature, distanceTo(creature)) }

collect takes a PartialFunction and will only return values at which this function is defined, so creature == this will not be returned.

And also you can replace

case creaturesWithDistance if creaturesWithDistance.isEmpty => None

with

case Seq() => None