4
votes

Suppose I want to write a function def pair(s:String):Option[(String, String)] to convert a string into a key-value pair in Scala. The string should look like "<key>:<value>".

How would you correct the solution below ?

def pair(s:String) = {
  val a = s.split(":"); if (a.length == 2) Some((a(0).trim, a(1).trim)) else None
}
2

2 Answers

9
votes

Two approaches I personally find a little nicer:

def pair(s: String) = s.split(":") match {
  case Array(k, v) => Some(k.trim -> v.trim)
  case _ => None
}

Or using Scala's handy regular expression extractors:

val Pair = """\s*([^\s:]+)\s*:\s*([^\s:]+)\s*""".r

def pair(s: String) = s match {
  case Pair(k, v) => Some(k -> v)
  case _ => None
}

But yeah, yours isn't that bad either.

5
votes

I'd remove the semicolon and make it two lines. Otherwise it's a completely fine functional code.

On the second glance pattern matching seems a bit more appropriate:

def pair(s:String) = 
  s.split(":") match {
    case Array(a, b) => Some((a, b))
    case _ => None
  } 

The reason why pattern matching fits better in this case is that you use if-else to emulate a datastructure deconstruction - the exact thing, which pattern matching is for. This drives you to a bit lower level of abstraction, more involved logic and more opportunities to implant bugs. Also as a consequence your implementation relies on partial functions as in a(0), which is always a discouraged practice.

Partial function is a function which is not defined for the whole domain of its input. In your case it means that calling a(0) on an empty array will result in an exception, but not in a meaningful functional result. In this particular example you are protected from that scenario by the if condition, but in more involved situations if this condition is placed outside of the function body, a use of partial function becomes a potential bug. That's why it is discouraged as a general practice.

But I also must mention that you should not frown upon the if-else construct generally. It is a perfectly valid functional expression which has its uses. It's just that in this particular example it was a suboptimal approach.