Scala: Try .getOrElse vs if/else

Question

I'm a fairly new Scala developer. I am an experienced Java developer and so far I've been enjoying Scala's simplicity. I really like the functional constructs and quite often they force you to write cleaner code. However recently I noticed due to comfort and simplicity I end up using constructs I wouldn't necessarily use in Java and would actually be considered a bad practice e.g.

private def convertStringToSourceIds(value: String) : Seq[Integer] = {
    Try(value.split(",").toSeq.map(convertToSourceId(_))).getOrElse(Seq())
}

The same code snippet can be written as

private def convertStringToSourceIds(value: String) : Seq[Integer] = {
    if(value!=null) value.split(",").toSeq.map(convertToSourceId(_)) else Seq()
}

A part of me realizes that the Try/getOrElse block is designed with Options in mind but quite often it makes code more readable and handles cases you might have missed (which of course isn't always a good thing).

I would be interested to know what is the opinion of an experienced Scala developer on the matter.

Bruno Grieder · Accepted Answer · 2015-12-15T17:13:54.333

4

I am not claiming any "experience" title but I much prefer your second construct for a few reasons

Throwing an exception (an NPE in this case) is expensive and best avoided; it should remain just that, exceptional
if are expressions in Scala, which avoids declaring "dangling" variables to hold the result of the test (just like ternary operators). Alternatively the match..case construct provides for very readable code.
I would personally return an Option[Seq[Integer]] to "pass back" the information that the values was null and facilitate further chaining of your function.

Something like

private def convertStringToSourceIds(value: String) : Option[Seq[Integer]] = value match {
    case null => None
    case _ => Some(value.split(",").map(convertToSourceId(_)))
}

note 1: not sure you need the toSeq

note 2: for good or bad, looks a bit Haskellish

The combination of Scala + FP makes it almost doubly certain you will get different opinions :)

Edit Please read comments below for additional reasons and alternatives, i.e,

def convertStringToSourceIds(value: String): Option[Array[String]] = Option(value).map(_.split(",").map(convertToSourceId(_)))

edited Dec 15 '15 at 17:13

answered Dec 15 '15 at 10:52

Bruno Grieder

28,128
8
69
101

5

it is unnecessary to match it, because `Option` does it automatically, just `Option(value).map(convertToSourceId(_.split(",")))` is needed. – Alexandr Dorokhin Dec 15 '15 at 10:59
Thank you a lot, it makes sense. You have several ways to do something correctly in Scala and it can be confusing. – George Kouzmov Dec 15 '15 at 15:01
`Throwing an exception (an NPE in this case) is expensive and best avoided`. I'd stress the importance of **losing** Referential Transparency when using exceptions (via http://programmers.stackexchange.com/questions/223329/side-effects-breaking-referential-transparency), not the *performance effect* (which I interpreted from your usage of *expensive*. – Kevin Meredith Dec 15 '15 at 16:34
@KevinMeredith Right, it is best avoided for its potential side effects and as a poor mechanism to handle program control flow. It can also be slow: http://stackoverflow.com/questions/299068/how-slow-are-java-exceptions – Bruno Grieder Dec 15 '15 at 16:47

score 3 · Answer 2 · answered Dec 15 '15 at 10:53

If you can, use Options instead of null to show when a value is missing.

Assuming that you can't use Options, a more readable way to handle this could be

private def convertStringToSourceIds(value: String) : Seq[Integer] = value match {
    case null => Seq();
    case s => s.split(",").toSeq.map(convertToSourceId(_));
  }

Scala: Try .getOrElse vs if/else

2 Answers2

Linked