Idiomatic Scala try option code block

Basically, this function returns a list of flights for an airline. But if the airline doesn't exist, I want to throw some custom runtime exception. If that airline does exist, I want to return a list of flights (which could potentially be empty).

Is this is the standard way of doing things in Scala, or is there a better way to write this code?

Coming from a Java background, I'm still trying to get to grips with the Scala way of doing things.

def getFlightsForAirline(id:Long):Try[Option[List[Flight]]] = {
     val maybeAirline:Option[Airline] = Airline.getById(id)

     maybeAirline match {
       case Some(airline) => {
          Success(Flight.getFlightsByAirline(airline))
       }
       case None => Failure(new RecordNotFoundException("Couldnt find airline"))
     }
   }

-
The maybeAirline variable isn't necessary. Note that the type annotation in the declaration is optional, and can easily be inferred by the compiler. So I would change that to:

def getFlightsForAirline(id: Long): Try[Option[List[Flight]]] =
  Airline.getById(id) match {
    case Some(airline) => Success(Flight.getFlightsByAirline(airline))
    case None => Failure(new RecordNotFoundException("Couldnt find airline"))
  }

Notice that the Scala style guide suggests putting a space after the colon in a type annotation – name: Type instead of name:Type.

-
Every time you destructure an Option with match/case, they kill a puppy because you should have probably used map. Well, almost every time, because the Failure is an important part of this function's behaviour. But this still is somewhat of a code smell.

-
The return type Try[Option[List[Flight]]] strikes me as rather weird: What is the difference between Success(Some(List())) and Success(None())? I guess the latter might occur if getFlightsByAirline isn't passed a valid airline.

Try, Option and List are essentially the same thing (either you get a result, or there isn't anything there: Failure, None or empty List). Therefore it does not make much sense to nest them, and it would be better to return a Try[List[Flight]]. This could be implemented along the lines of:

def getFlightsForAirline(id: Long): Try[List[Flight]] =
  Airline.getById(id) flatMap { Flight.getFlightsByAirline(_) } match {
    case Some(flights) => Success(flights)
    case None => Failure(new RecordNotFoundException("Couldnt find airline"))
  }

This way, the Failure is returned if either of the getXByY methods returns None.

Interestingly, there is no builtin way to transform an Option to a Try (along the lines of maybe toTry { MyException("Reason") }) while there is a try.toOption method.

-
Your RecordNotFoundException will have a much more Scala-ish vibe if you use a case class here. This way, you can omit the new to create a new instance, and just write: Failure(RecordNotFoundException("..."))

StackExchange Code Review Q#37321, answer score: 7