Mateusz Chrzaszcz Mateusz Chrzaszcz - 4 years ago 72
Scala Question

Is using Optional in Scala's case classes and classes fields a code smell?

There were already quite a few discussions on Stackoverflow about proper ways of using Optional in Java (Discussions like this one, or this)

As of now, using Optional for class members in Java is widely recognized as a code smell and even discouraged by fact that it deliberately does not implement Serializable interface. Also, we should avoid it in DTOs, constructors and methods' input parameters. From OOP point of view everything I have read so far about Optional appeals to my reason.

My question is, does FP side of Scala change something in a way we should use Optional ? Especially since implementation of Optional in Scala seems to be way richer. I have found plenty of articles describing how to use it in Scala, but not a single one that exhaust topic when should I use it and when should I not.

Answer Source

Short answer

Even though several well established libraries (e.g. ScalaTest) define classes with Option fields, the latter, IMO, tend to be a code smell, as they often try to do too much for their own good.

In many cases, eliminating Option fields by defining a small hierarchy of more focused types (instead of just a single type) is both easy and beneficial.

An example

The domain

Consider a business domain dealing with accounts. An account starts its life one day as an open account, but may eventually be closed. Accounts, among other data, contains the dates on which they were open and closed, where applicable.

Using an Option field

Here is an implementation of an account, using an Option field:

final case class Account(openOn: LocalDate, closedOn: Option[LocalDate], ...)

We also have an account service, which defines a close method:

trait AccountService {
  def close(account: Account): Account
}

This approach is problematic, for a number of reasons. One problem is that Account isn't particularly performant: because closedOn is a "boxed" type, you have one level of indirection too many, so to speak. Moreover, Account's memory footprint is less than ideal: a "closed account" contains a pretty uninteresting value (None), which is a waste of space.

Another, more serious, problem is that the close method cannot enforce, at the type level, that the parameter be an "open account" and the result be a "closed account". You would have to write tests to check that this business rule is enforced by your implementation.

Using a small ADT (and eschewing Option fields)

Consider the following alternative design:

sealed trait Account { ... }

final case class OpenAccount(openOn: LocalDate, ...) extends Account

final case class ClosedAccount(openOn: LocalDate, closedOn: LocalDate, ...) extends Account

This small ADT remedies the performance problem, but there is more... You can now encode the business rule at the type level! This is an example of making illegal states unrepresentable. As a result, your service's API becomes more expressive and harder to misuse:

trait AccountService {
  def close(account: OpenAccount): ClosedAccount
}

This example may be sufficient to convince you that the second approach is preferable, and that Option fields are best avoided (or, at least, used sparingly).

Recommended from our users: Dynamic Network Monitoring from WhatsUp Gold from IPSwitch. Free Download