CODEWITHSUNDEEP
scalacoding-style
Chris
Chris

Reputation: 435

Abuse of match?

Would you consider the following block of code match abuse and if so what's a more elegant way to do it without a big if-else-if block?

def sum(base: Int, xs: List[Int]): Int = {
  base match {
    case 0 => 1
    case _ if (base < 0) => 0
    case _ if (xs.isEmpty) => 0
    case _ => xs.sum
  }
}

Upvotes: 6

Views: 2422

Answers (2)

0__
0__

Reputation: 67280

No. Why abuse? It's fairly readable IMO...

The problem I see is that money match ... is fairly arbitrary (you only use the direct pattern in the first case); a complete "abuse" would start like

() match {
  case _ if (money == 0) => 1
  ...

So perhaps stick to if-else; you can combine the second and third condition (if( money < 0 || coins.isEmpty ) ...)

Also note that although you "know" in the end that coins is not empty and thus may "safely" call head and tail on it, this is a typical source of unexpected runtime errors. The advantage of coins match { case Nil => ...; case head :: tail => ...} is that you cannot make such a mistake.

Upvotes: 2

dhg
dhg

Reputation: 52681

Yes, this an abuse of match. You've basically just written a big if-else-if block, but in a more awkward form. What's wrong with if-statements?

I think it's much cleaner to just write this:

def countChange(money: Int, coins: List[Int]): Int = {
  if(money == 0) 1
  else if (money < 0) 0
  else if (coins.isEmpty) 0
  else countChange(money, coins.tail) + countChange(money - coins.head, coins)
}

If you want to stick with a match, you can move more of the checking into the match itself, so that it's actually doing something:

def countChange(money: Int, coins: List[Int]): Int = {
  (money, coins) match {
    case (0, _) => 1
    case _ if (money < 0) => 0
    case (_, Nil) => 0
    case (_, coinsHead :: coinsTail) => countChange(money, coinsTail) + countChange(money - coinsHead, coins)
  }
}

Upvotes: 19

Related Questions