Idiomatic way to use Options's in Scala

Question

I am converting some Java code to Scala, trying to make the code as idiomatic as possible.

So, I now have some code using Options instead of nullable values, and I wonder whether things are Scalaiish, or whether I'm wrong. So, could you guys please criticize the following snippet of code?

The areas in which I am specifically looking for feedback are:

• The use of a companion object as a factory, giving 2 options depending on whether we want to pass Options or Strings: is the String constructor fine, or should we always expose the fact that it is an Option?
• The use of preconditions: are there better ways to assert the fact that alpha3Code and name are mandatory, and a non-null option mustbe passed for alpha2Code? (I am resorting to Guava for the string utils, as I haven't found anything in the Scala API)
• The implementation of hashCode, equals and toString. equals and toString delegate to Guava again, whereas equals uses pattern matching. Is there a more Scala-ish way?
• I know I could have used Case classes, which would have created default implementations, but I am mostly interested in learning how I should implement those for the cases where case classes cannot be used.

    package com.sirika.openplacesearch.api.language
    
    import com.google.common.base.Objects
    import com.google.common.base.Strings
    
    object Language {
        def apply(name : String, alpha3Code : String, alpha2Code : Option[String]) = new Language(name, alpha3Code, alpha2Code)
        def apply(name : String, alpha3Code : String, alpha2Code : String = null) = new Language(name, alpha3Code, Option(alpha2Code))
        def unapply(l : Language) = Some(l.name, l.alpha3Code, l.alpha2Code )
    }
    
    
    class Language(val name : String, val alpha3Code : String, val alpha2Code : Option[String]) {
        require(!Strings.isNullOrEmpty(alpha3Code))
        require(!Strings.isNullOrEmpty(name))
        require(alpha2Code != null)
        
        override def hashCode(): Int = Objects.hashCode(alpha3Code)
        
                override def equals(other: Any): Boolean = other match {
            case that: Language => this.alpha3Code == that.alpha3Code
            case _ => false
        }
    
        override def toString() : String = Objects.toStringHelper(this)
            .add("name", name)    
            .add("alpha3", alpha3Code)
            .add("alpha2", alpha2Code)
            .toString()
    }

Answer 1

I think you should expose only Option[String] in factory method. For example I, as a user of your library, will also ask myself question which factory method I should use. And most probably I will use Option.

Scala gives us enough tools to make our lifes easier. For example you can use default for option like this:

def apply(name: String, alpha3Code: String, alpha2Code: Option[String] = None) = 
 new Language(name, alpha3Code, alpha2Code)

If I, again as user of your library, want to pass just string without wrapping it in Some each time, I can write my own implicit conversion like this:

implicit def anyToOption[T](t: T): Option[T] = Some(t)

or even (if I personally use nulls):

implicit def anyToOption[T](t: T): Option[T] = 
 if (t == null) None else Some(t)

But I believe, if you enforce option, it will make your API more solid and clear.

Answer 2

You should avoid null unless there's a very good reason not to. As it is, you could have just written this:

def apply(name : String, alpha3Code : String, alpha2Code : String) = new Language(name, alpha3Code, Option(alpha2Code))
def apply(name : String, alpha3Code : String) = new Language(name, alpha3Code, None)

The preconditions are fine. You could write it like this:

require(Option(alpha3Code) exists (_.nonEmpty))
require(Option(name) exists (_.nonEmpty))

Not necessarily an improvement, though.

A String has hashCode, so I don't understand why you are calling another method to generate a hash code instead of just calling alpha3Code.hashCode. I do think there's something in the Scala API, though. Not sure.

The equals code should have a canEqual method, unless you make your class sealed or final. Pattern match is pretty much the way to do it, though you could have written it like this given the presence of an extractor:

case Language(_, `alpha3Code`, _) => true

But the way you wrote it is pretty much the way it is usually written.