10
Feb 10

Avoid using nulls in Scala

Scala’s handling of null’s mixed with implicit casting is quite tricky. I learned the hard way today and it took hours to figure out what was going on. First I thought it was a bug, but then someone pointed out how implicit casting effect null method parameters.

The bottom line is: DO NOT USE NULLs unless you are utilizing java libraries and have no choice. Use Option instead, with Some() or None().

The problem is best described with code…

  def checkNullOrEmpty(v:Seq[Any]):Boolean = {
    println("Class:"+v.getClass)
    return (v != null) && !v.isEmpty
  }

  case class Race(val event:String, val protocol:String) {
    println("Event:"+event+", protocol:"+protocol)
    assert(checkNullOrEmpty(event))
    assert(checkNullOrEmpty(protocol))
  }

  val t = new Race(null, null)

Pasting the above into REPL yields the following result…

Event:null, protocol:null
Class:class scala.collection.immutable.WrappedString
java.lang.NullPointerException
    at scala.Proxy$class.toString(Proxy.scala:29)
    at scala.collection.immutable.WrappedString.toString(WrappedString.scala:22)
    at scala.collection.immutable.StringLike$class.length(StringLike.scala:48)
    at scala.collection.immutable.WrappedString.length(WrappedString.scala:22)
    at scala.collection.IndexedSeqLike$class.isEmpty(IndexedSeqLike.scala:81)
    at scala.collection.immutable.WrappedString.isEmpty(WrappedString.scala:22)
    at .checkNullOrEmpty(<console>:6)
    .....

So why is NPE thrown at this breakpoint return (v != null) && !v.isEmpty?

So let’s look further into the output. When a Race instance is created, the constructor values are initialized to null. Inside the constructor we print this out and verify that values are in fact null. When We get to checkNullOrEmpty method call, the class of v is WrappedString and though the object is no longer null. In Java the call to getClass would fail, as v would be null, in Scala it’s casted (converted) to WrappedString.

This happens through Scala’s implicit conversions. The checkNullOrempty method expects a Seq. Although Seq is not a superclass or interface of String. So when we create the Race instance and specify both event and protocol as null, they are still String types with a null reference. Using say event or protocol as an argument to checkNullOrEmpty yields an implicit conversion. Why? Well, in Java the compilation would fail, since the Seq trait is not a part of the inheritance hierarchy of String, but in Scala, it succeeds, as Scala finds an implicit conversion method to convert String to WrappedString. This method is defined in Predef object. We know the Predef is imported by default into all Scala classes. Predef extends LowPriorityImplicits class, which defines this implicit conversion implicit def wrapString(s: String): WrappedString = new WrappedString(s). So basically Scala decides that the best way to convert the String type into Seq[Any] is by using this implicit conversion. So it wraps the null value with the WrappedString.

This causes two issues… First, the not null check no longer works, as the object is not null, due to the fact that it’s an instance of WrappedString, so (v != null) is true. Since that passes, it then executes the RHS of && operator and then tries to infer on !v.isEmpty, which throws the NPE, as the underlying String value wrapped is null.

I’m not necessarily sure whether this is a bug, feature, or maybe there is no real consensus on how null should be handled, but as you see this causes issues and should either be avoided through avoiding null and using Option instead. If you are using java libs that return nulls, wrapping the return value in Option might be a good idea, before proceeding any further.

Tags: ,

10 comments

  1. In my mind the implicit conversion is the bug. A null-reference String should not be turned into a WrappedString that is no longer null. If that was the case, there would not be any unexpected behaviour.

  2. In my mind the implicit conversion is the bug. A null-reference String should not be turned into a WrappedString that is no longer null. If that was the case, there would not be any unexpected behaviour.

  3. That implicit conversion makes no sense.

  4. That implicit conversion makes no sense.

  5. I tend to agree; implicit wrappers need to check that they apply only to non-null values. That needs to be done in the classes defining those wrappers.

  6. I tend to agree; implicit wrappers need to check that they apply only to non-null values. That needs to be done in the classes defining those wrappers.

  7. Yes that’s a good option, but being that anyone can use implicits I wonder if this check shouldn’t be done by the compiler? I can’t think of any reason why one would want an implicit to wrap a null. With this being a compiler check it will prevent any surprises from 3rd party defined implicits.

  8. Yes that’s a good option, but being that anyone can use implicits I wonder if this check shouldn’t be done by the compiler? I can’t think of any reason why one would want an implicit to wrap a null. With this being a compiler check it will prevent any surprises from 3rd party defined implicits.

  9. I think everybody agrees that null is a big wart in the Scala’s type system but is there for Java compatibility. As you said, idiomatic Scala uses Option[T] instead. Nulls are a runtime event, so the compiler cannot infer much, ( unless you use Option :)) In this particular case, maybe WrappedString should throw a “null parameter exception” or check for null in isEmpty, but I agree that implicits can have nasty interactions with nulls. bottom line: nulls are bad, mmmmkay?

  10. I think everybody agrees that null is a big wart in the Scala’s type system but is there for Java compatibility. As you said, idiomatic Scala uses Option[T] instead. Nulls are a runtime event, so the compiler cannot infer much, ( unless you use Option :)) In this particular case, maybe WrappedString should throw a “null parameter exception” or check for null in isEmpty, but I agree that implicits can have nasty interactions with nulls. bottom line: nulls are bad, mmmmkay?

Leave a comment