Using Option all over the place feels a bit awkward. Am I doing something wrong?

Question

As a result of articles I read about the Option class which helps you avoid NullPointerException's, I started to use it all over the place. Imagine something like this:

var file:Option[File] = None

and later when I use it:

val actualFile = file.getOrElse(new File("nonexisting"))
if(actualFile.getName.equals("nonexisting")) { // instead of null checking

}
else { // value of file was good

}

Doing stuff like this doesn't feel all that "right" to me. I also noticed that .get has become deprecated. . Is this sort of stuff what you guys are doing with Option's too, or am I going the wrong way?

Answer 1

It's generally not a good idea to return Option and then use getOrElse to produce some sentinel value that means "not found". That's what Option is designed for: to signify that a value is not found!

Option really shows its power when used in conjunction with functional programming constructs like map and foreach. This is most potent when dealing with multiple options. For example, suppose I write a method that takes a string and gives me back a file, but only if the file exists and is a file not a directory:

import java.io._;
def niceFile1(s: String): File = {
  val f = new File(s);
  if (f.exists && !f.isDirectory) f else null
}
def niceFile2(s: String): Option[File] = {
  val f = new File(s);
  if (f.exists && !f.isDirectory) Some(f) else None
}

So far, using null is easier--at least until you forget that this might give you null and you get a NPE. Anyway, let's now try to use it.

def niceFopen1(s: String) = {
  val f = niceFile1(s);
  if (f!=null) new FileInputStream(f) else null;
}
def niceFopen2(s: String) = niceFile2(s).map(f => new FileInputStream(f))

Look what happened! In the former case, we had to do logic tests by hand and create temporary variables. Ugh! In the second case, map did all the dirty work for us: None was mapped to None, and Some(file) was mapped to Some(fileinputstream). Easy!

But it gets better yet. Maybe we want to find the size of a whole bunch of files:

def totalSize2(ss: Seq[String]) = {
  (0L /: ss.flatMap(niceFile2)){(sum,f) => sum+f.length}
}

Wait, what is going on here--what about all the None? Don't we have to pay attention and handle them somehow? Well, that's where flatMap comes in: it joins together all the answers into a single list. None is an answer of length zero, so it ignores it. Some(f) has one answer--f--so it puts it in the list. Then we use a fold to add up all the lengths--now that all the elements in the list are valid. Pretty nice!

Answer 2

It's a good idea to not resolve the value of the Option, but to apply logic to whatever is it contains:

findFile.foreach(process(_))

Basically this processes a File if one is found and does nothing otherwise (and is equivalent to Thomas' first for comprehension because for compiles to a call to foreach). It is a more concise version of:

findFile match {
  case Some(f) => process(f)
  case None =>
}

What's more, the great thing about this is that you can chain operations, something like:

(findLiveFile orElse fileBackupFile orElse findTempFile).foreach(process(_)

Answer 3

In most cases you would use pattern matching

file match {
   case Some(f) => { .. } //file is there
   case _ => { .. } //file is not there 
}

If you're only interested in the file if it is there you can use a for expression

for(f <- file) { //file is there 
}

You can then chain for expressions to work on multiple levels on containers

for{ 
  option <- List(Some(1), None, Some(2))
  f <- option
} yield f

res0: List[Int] = List(1, 2)

Alternatively you can use isDefined and get:

if(option.isDefined) {
   val x = option.get;
} else {
}

get is not deprecated in Scala 2.8.0.Beta-1.

Answer 4

Here's an alternative:

var file:Option[File] = None
// ...
file map (new File(_)) foreach { fh =>
  // ...
}

However, if you need to do something if the file exists, and something else if it does not, a match statement is more appropriate:

var file:Option[File] = None
// ...
file map (new File(_)) match {
  case Some(fh) =>
    // ...
  case None =>
    // ...
}

That's pretty much the same as an if statement, but I like it's chaining nature better for things like Option, where I want to extract a value as well.

Answer 5

Mostly restating what everyone is saying, but I think there's at least 4 different situations you will encounter when you have a nullable var like this:

1. The file must not be null

load(file.get())

Will throw an exception when you try to actually use the file. Fail fast, yay!

2. The file is null, but in this case we have a sensible default:

load(file.getOrElse(new File("/defaultFile")))

3. Two branches of logic based on whether the file exists:

  file match {
    case Some(f) => { .. } //file is there
    case _ => { .. } //file is not there 
  }

4. No-op if the file is null, aka silently fail. I don't think this one is very often preferable for me in real life:

for (f <- file) {
//do some stuff
}

Or, maybe more clearly?

if (f.isDefined) {
  //do some stuff
}