2
\$\begingroup\$

Summary:

I've implemented what I think is a complete Java implementation of the Scala Option class. Could you please review and let me know if I have correctly and completely implemented it? And if not, could you please indicate where I have security, multi-threading, persistence, etc. defects?

Details:

I'm learning Scala, but still not able to use it in my projects at work. However, I have fallen in love with Scala's Option class. It's the equivalent to Haskell's Maybe. It's a great way to consistently implement the "null object" pattern; i.e. type safely getting rid of tons of potentially hidden NullPointerExceptions.

While working on bringing some legacy Java code forward in time (written by me +10 years ago), I kept wanting to use something like Scala's Option, only written in Java for Java. Thus began my hunt for an effective Java version of Scala's option class.

First, I googled and found something simple from Daniel Spiewak written in 2008: The Option Pattern.

A comment by Tony Morris on Daniel's blog article (above) lead me to an article by Tony Morris where Daniel's idea was made a bit more "complete", also adding to it a bit more complexity (at least to me): Maybe in Java.

Then, while doing further research to try and find something already written, rather than my writing it myself, I found this one. Written earlier this year (2012), it seemed quite up-to-date: Another Scala option in Java.

However, once I brought this latest code into my project to use, it started putting yellow squigglies under any references to None. Basically, the singleton the author used was leaking "raw types" into my code generating warnings. That was particularly annoying as I had just finished eliminating all the raw type code warnings from this particular code base the previous week.

I was now hooked on getting something working that might resemble a work that could/would appear in the high-quality Java libraries, eliminating raw type leakage, adding things like serialization support, etc.

Below is the result as three separate Java class files:

  1. Option - public interface
  2. Optional - factory for safely producing instances of Some and None
  3. Main - JUnit4 test suite to ensure proper functionality

Please review and then give me any feedback and/or corrections you are willing to contribute.

1. Interface Option - public interface

package org.public_domain.option;

import java.util.Iterator;
import java.io.Serializable;

public interface Option<T> extends Iterable<T>, Serializable {
  @Override
  Iterator<T> iterator();
  T get();
  T getOrElse(T value);
}

2. Class Optional - Option instance factory

package org.public_domain.option;

import java.util.ArrayList;
import java.util.Collections;
import java.util.Iterator;
import java.util.List;

public final class Optional<T> {

  @SuppressWarnings("rawtypes")
  private static volatile Option NONE_SINGLETON = new OptionImpl();

  public static <T> Option<T> getSome(T value) {
    if (value == null) {
      throw new NullPointerException("value must not be null");
    }
    return new OptionImpl<T>(value);
  }

  @SuppressWarnings({"unchecked", "cast"})
  public static <T> Option<T> getNone() {
    return (Option<T>)getNoneSingleton();
  }

  @SuppressWarnings("unchecked")
  public static <T> Option<T> getOptionWithNullAsNone(T value) {
    return
          (value != null)
        ? new OptionImpl<T>(value)
        : (Option<T>)getNoneSingleton()
        ;
  }

  public static <T> Option<T> getOptionWithNullAsValidValue(T value) {
    return new OptionImpl<T>(value);
  }

  @SuppressWarnings("rawtypes")
  static Option getNoneSingleton() {
    return NONE_SINGLETON;
  }

  private Optional() {
    //no instances may be created
  }

  private static final class OptionImpl<T> implements Option<T> {
    private static final long serialVersionUID = -5019534835296036482L;

    private List<T> values; //contains exactly 0 or 1 value

    protected OptionImpl() {
      if (getNoneSingleton() != null) {
        throw new IllegalStateException("NONE_SINGLETON already defined");
      }
      this.values = Collections.<T>emptyList();
    }

    protected OptionImpl(T value) {
      List<T> temp = new ArrayList<T>(1);
      temp.add(value); //even if it might be a null
      this.values = temp;
    }

    @Override
    public int hashCode() {
      return this.values.hashCode();
    }

    @Override
    public boolean equals(Object o) {
      boolean result = (o == this);

      if (!result && (o instanceof OptionImpl<?>)) {
        result = this.values.equals(((OptionImpl<?>)o).values);
      }

      return result;
    }

    protected Object readResolve() {
      Object result = this;

      if (this.values.isEmpty()) {
          result = getNoneSingleton();
      }

      return result;
    }

    @Override
    public Iterator<T> iterator() {
      return
            (!this.values.isEmpty())
          ? Collections.unmodifiableList(new ArrayList<T>(this.values)).iterator()
          : this.values.iterator()
          ;
    }

    @Override
    public T get() {
      if (this.values.isEmpty()) {
        throw new UnsupportedOperationException("Invalid to attempt to use get() on None");
      }
      return this.values.get(0);
    }

    @Override
    public T getOrElse(T valueArg) {
      return
            (!this.values.isEmpty())
          ? get()
          : valueArg
          ;
    }
  }
}

3. Class Main - JUnit4 test suite

package org.public_domain.option.test;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotSame;
import static org.junit.Assert.assertSame;
import static org.junit.Assert.assertTrue;

import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.ObjectInputStream;
import java.io.ObjectOutputStream;
import java.util.HashMap;
import java.util.Iterator;
import java.util.Map;

import org.junit.Test;
import org.public_domain.option.Option;
import org.public_domain.option.Optional;

public class Main {

  @Test
  public void simpleUseCasesSome() {
    String value = "simpleUseCases";
    String valueOther = "simpleUseCases Other";
    //Some
    Option<String> optionSome = Optional.getSome(value);
    assertEquals(value, optionSome.get());
    assertEquals(value, optionSome.getOrElse(valueOther));
    assertNotSame(valueOther, optionSome.get());
    assertNotSame(valueOther, optionSome.getOrElse(valueOther));
    //validate simple iterator state (each call is a newly created iterator)
    assertTrue(optionSome.iterator().hasNext());
    assertEquals(value, optionSome.iterator().next());
    assertNotSame(valueOther, optionSome.iterator().next());
    //ensure iterator exhausted after single return value (all on the same iterator)
    Iterator<String> iterator = optionSome.iterator();
    assertTrue(iterator.hasNext());
    assertEquals(value, iterator.next());
    assertFalse(iterator.hasNext());
  }

  @Test
  public void simpleUseCasesNone() {
    String value = "simpleUseCases";
    String valueOther = "simpleUseCases Other";
    Option<String> optionNone = Optional.getNone();
    assertEquals(valueOther, optionNone.getOrElse(valueOther));
    assertNotSame(value, optionNone.getOrElse(valueOther));
    //ensure iterator is already exhausted
    assertFalse(optionNone.iterator().hasNext());
  }

  @Test (expected=NullPointerException.class)
  public void simpleInvalidUseCaseSomePassedNull() {
    @SuppressWarnings("unused")
    Option<String> optionSome = Optional.getSome(null);
  }

  @Test (expected=UnsupportedOperationException.class)
  public void simpleInvalidUseCaseNoneGet() {
    Option<String> optionNone = Optional.getNone();
    @SuppressWarnings("unused")
    String value = optionNone.get();
  }

  @Test
  public void simpleUseCaseEquals() {
    String value = "simpleUseCases";
    String valueSame = value;
    String valueDifferent = "simpleUseCases Other";
    Option<String> optionSome = Optional.getSome(value);
    Option<String> optionSomeSame = Optional.getSome(valueSame);
    Option<String> optionSomeDifferent = Optional.getSome(valueDifferent);
    Option<String> optionNone = Optional.getNone();
    Option<String> optionNoneSame = Optional.getNone();
    //Some - self-consistency
    assertSame(optionSome, optionSome); //identity check
    assertEquals(optionSome, optionSomeSame); //content check
    assertEquals(optionSomeSame, optionSome); //symmetry check
    assertNotSame(optionSome, optionSomeDifferent); //identity and content check
    assertNotSame(optionSomeDifferent, optionSome); //symmetry check
    //None - self-consistency
    assertSame(optionNone, optionNoneSame); //identity check
    assertSame(optionNoneSame, optionNone); //symmetry check
    //Some-vs-None consistency
    assertNotSame(optionSome, optionNone);  //identity check
    assertNotSame(optionNone, optionSome);  //symmetry check
  }

  @Test
  public void useCaseSomeWithNullAsNone() {
    String value = null;
    String valueSame = value;
    String valueDifferent = "simpleUseCases";
    Option<String> option = Optional.getOptionWithNullAsNone(value);
    Option<String> optionSame = Optional.getOptionWithNullAsNone(valueSame);
    Option<String> optionDifferent = Optional.getOptionWithNullAsNone(valueDifferent);
    //Some - self-consistency
    assertSame(option, option); //identity check
    assertEquals(option, optionSame); //content check
    assertEquals(optionSame, option); //symmetry check
    assertNotSame(option, optionDifferent); //identity and content check
    assertNotSame(optionDifferent, option); //symmetry check
    //None consistency
    Option<String> optionNone = Optional.getNone();
    assertSame(option, optionNone);
    assertSame(optionNone, option); //symmetry check
  }

  @Test
  public void useCaseSomeWithNullAsValidValue() {
    String value = null;
    String valueSame = value;
    String valueDifferent = "simpleUseCases";
    Option<String> option = Optional.getOptionWithNullAsValidValue(value);
    Option<String> optionSame = Optional.getOptionWithNullAsValidValue(valueSame);
    Option<String> optionDifferent = Optional.getOptionWithNullAsValidValue(valueDifferent);
    //Some - self-consistency
    assertSame(option, option); //identity check
    assertEquals(option, optionSame); //content check
    assertEquals(optionSame, option); //symmetry check
    assertNotSame(option, optionDifferent); //identity and content check
    assertNotSame(optionDifferent, option); //symmetry check
    //None consistency
    Option<String> optionNone = Optional.getNone();
    assertNotSame(option, optionNone);
    assertNotSame(optionNone, option); //symmetry check
  }

  private byte[] transformToByteArray(Object root) {
      ByteArrayOutputStream baos = new ByteArrayOutputStream(65536);
      try {
        ObjectOutputStream oos = new ObjectOutputStream(baos);
        oos.writeObject(root);
      } catch (IOException e) {
        e.printStackTrace();
      }
      return baos.toByteArray();
  }

  private Object transformFromByteArray(byte[] content) {
    Object result = null;

    ByteArrayInputStream bais = new ByteArrayInputStream(content);
    ObjectInputStream ois;
    try {
      ois = new ObjectInputStream(bais);
      result = ois.readObject();
    } catch (IOException e) {
      e.printStackTrace();
    } catch (ClassNotFoundException e) {
      e.printStackTrace();
    }
    return result;
  }

  @Test
  public void useCaseSerialzation() {
    String value = "simpleUseCases";
    String valueSame = value;
    String valueDifferent = "simpleUseCases Other";
    Option<String> optionSome = Optional.getSome(value);
    Option<String> optionSomeSame = Optional.getSome(valueSame);
    Option<String> optionSomeDifferent = Optional.getSome(valueDifferent);
    Option<String> optionNone = Optional.getNone();
    Option<String> optionNoneSame = Optional.getNone();
    Map<String, Option<String>> dataIn = new HashMap<String, Option<String>>();
    dataIn.put("optionSome", optionSome);
    dataIn.put("optionSomeSame", optionSomeSame);
    dataIn.put("optionSomeDifferent", optionSomeDifferent);
    dataIn.put("optionNone", optionNone);
    dataIn.put("optionNoneSame", optionNoneSame);
    byte[] dataInAsByteArray = transformToByteArray(dataIn);
    @SuppressWarnings("unchecked")
    Map<String, Option<String>> dataOut = (Map<String, Option<String>>)transformFromByteArray(dataInAsByteArray);
    assertEquals(optionSome, dataOut.get("optionSome"));
    assertEquals(optionSomeSame, dataOut.get("optionSomeSame"));
    assertEquals(optionSomeDifferent, dataOut.get("optionSomeDifferent"));
    assertSame(optionNone, dataOut.get("optionNone"));
    assertSame(optionNoneSame, dataOut.get("optionNoneSame"));
  }
}
\$\endgroup\$
  • \$\begingroup\$ Here's a great comment by Daniel Spiewak regarding the proper context and effective use of Scala's Option: beust.com/weblog/2010/07/28/… \$\endgroup\$ – chaotic3quilibrium Jun 5 '12 at 19:53
  • 1
    \$\begingroup\$ FYI: The "Functional Java" library has a Option implementation. \$\endgroup\$ – RoToRa Jun 6 '12 at 14:21
  • \$\begingroup\$ Awesome video covering Scala's Option described from the perspective of Monads: youtube.com/watch?v=Mw_Jnn_Y5iA \$\endgroup\$ – chaotic3quilibrium Jun 10 '12 at 2:22
  • 1
    \$\begingroup\$ Options are monads and monads have certain laws they must follow (what methods they should have and those methods should do). If you want to learn more about the monad laws a great place to start is by checking out James Iry's series of posts "Monads are not Elephants" starting at james-iry.blogspot.com/2007/09/monads-are-elephants-part-1.html \$\endgroup\$ – sourcedelica Jun 23 '12 at 23:57
  • \$\begingroup\$ 3 years later, Java 8 has Optional<T>. What we need even more is something similar to Scala's Try[T]. \$\endgroup\$ – Tamoghna Chowdhury Jul 25 '17 at 14:52
1
\$\begingroup\$

If you want to guarantee serializability, the T type parameter must be Serializable, or your Option won't be:

public interface Option<T extends Serializable> extends Iterable<T>, Serializable

Also, the use of a public interface to declare Option allows anybody to create their own implementation. The code will break if it is given another implementation of the Option interface, because it assumes the only implementation is an OptionImpl.

Here's an alternate implementation much the same as yours that I'd come up with:

package util;

import java.util.Collections;
import java.util.Iterator;
import java.util.NoSuchElementException;

public abstract class Option<A> implements Iterable<A> {

  private Option() {}

  public abstract A get();
  public abstract A getOrElse(A defaultResult);
  public abstract Iterator<A> iterator();
  public abstract boolean match(Option<A> other);

  @SuppressWarnings("unchecked")
  public static <A> Option<A> Option(final A a) {
    return a == null? (Option<A>)None : Some(a);
  }

  public static <A> Option<A> Some(final A a) {
    return new _Some<A>(a);
  }

  @SuppressWarnings("unchecked")
  public static <A> Option<A> None() {
    return (Option<A>)None;
  }


  @SuppressWarnings("rawtypes")
  public static final Option None = new _None();


  private static final class _Some<A> extends Option<A> {
    private final A value;

    private _Some(A a) {
      if (a == null) throw new IllegalArgumentException("argument to Some may not be null");
      this.value = a;
    }

    @Override
    public A get() {
      return this.value;
    }

    @Override
    public A getOrElse(final A ignored) {
      return this.value;
    }

    @Override
    public Iterator<A> iterator() {
      return Collections.<A>singleton(this.value).iterator();
    }

    @Override
    public boolean match(final Option<A> other) {
      return other == None? false : value.equals( ((_Some<A>)other).value );
    }

    @Override
    @SuppressWarnings("unchecked")
    public boolean equals(final Object obj) {
      return obj instanceof Option? match((Option<A>)obj) : false;
    }

    @Override
    public int hashCode() {
      return this.value.hashCode();
    }

    @Override
    public String toString() {
      return "Some(" + this.value + ")";
    }
  }


  private static final class _None<A> extends Option<A> {
    private _None() {}

    @Override
    public A get() { throw new NoSuchElementException("None.get() called"); }

    @Override
    public A getOrElse(final A result) {
      return result;
    }

    @Override
    public Iterator<A> iterator() {
      return Collections.<A>emptyList().iterator();
    }

    @Override
    @SuppressWarnings("rawtypes")
    public boolean match(final Option other) {
      return other == None;
    }

    @Override
    public boolean equals(final Object obj) {
      return obj == this;
    }

    @Override
    public int hashCode() {
      return 0;
    }

    @Override
    public String toString() {
      return "None";
    }
  }
}

For usage, I use import static:

import util.Option;
import static util.Option.*;

Option<Boolean> selfDestruct = getSelfDestructSequence();
if (selfDestruct.match(Some(true)) {
  blowUpTheShip();
}

That way it reads similar to pattern matching and deconstruction, but of course it's really construction and equals(). And you can compare to None using ==.

The only thing I don't like about this is that the None singleton does give unchecked warnings on usage unless you use the factory method to return it. If only we had existential types, we could overcome that.

\$\endgroup\$
  • \$\begingroup\$ I wouldn't want to restrict my Option container to just holding a Serializable result, would I? When modeling this, I tried to follow the pattern set in the standard Java SE library for ArrayList/AbstractList/AbstractCollection. It appears that they will hold any Object. And then expect the client to handle any issues with Serialization in their own classes, should they need Java Serialization support. My implementation intended to just make handling Serialization transparent should a client want to use it. I may be missing something completely obvious, though. Thoughts? \$\endgroup\$ – chaotic3quilibrium Jun 6 '12 at 13:12
  • \$\begingroup\$ BTW, I really like your use of "Collections.<A>singleton(this.value)". I did not know about and am very please to learn about this method. I have updated my implementation to incorporate it above. \$\endgroup\$ – chaotic3quilibrium Jun 6 '12 at 13:22
  • 1
    \$\begingroup\$ Being a bit tired when responding, I took Serializable as being more of a contract than a possibility. Of course you can declare the Option as Serializable and it will be if its contents are, so I agree with your assessment. \$\endgroup\$ – WeaponsGrade Jun 6 '12 at 14:41
  • 1
    \$\begingroup\$ Re: match() vs. equals(), part of the goal was to make it read more like Scala. Of course, I'd also implement equals() and hashCode(), equals() almost being an alias for match() (match is type-safe, whereas equals cannot be). So part of the reasoning was type safety...the compiler won't let you call Option<A>.match(Option<B>). \$\endgroup\$ – WeaponsGrade Jun 6 '12 at 14:45
  • 1
    \$\begingroup\$ There, added equals and hashCode, as well as a missing factory method Option which will take an A and turn it into a None if null, or Some(a) otherwise. \$\endgroup\$ – WeaponsGrade Jun 6 '12 at 15:01
6
\$\begingroup\$

The main problem of your code is that it is written in Java. ;)

Unfortunately Java doesn't support Lambdas, Pattern-Matching and Type-Inference - three main points for effective using Options. Without these features you can't use Option as they should be used.

Option is a Monad but your implementation isn't and probably will it never be because Javas syntax is not powerful enough to allow Monad-handling in a useful manner. See the following code example:

val value = for {
  o <- Option(somePotenzialNullValue)
  a <- someOptionValue(o)
  b <- anotherOptionValue
  c = a operateOn b
  if c == anyValue
} yield c

How would you implement this in Java? Even if you use Javas foreach-loop this will always look ugly and not intuitive.

If you wanna know how Options work, how they can implemented in an effective way or to have some fun, than implement Option in Scala - or use Java8 as I did. ;)

Java8 will support Lambdas, thus it allows some useful code lifting. Here is my implementation: Java8 Option type (not a perfect implementation but it works)

\$\endgroup\$
  • \$\begingroup\$ LOL! I agree. However, while I am pre-Java 8 land, I still want to get even the minimal value out of the Java type system to catch possible NPE pathways. I get that this Option doesn't provide that entirely. No implementation of the pattern really can. However, if I use Option AND I avoid the use of Option.get(), then I will be more likely to think of and handle possible NPE pathways. In summary, my favorite summary of Option is this - think of it a Set (or List) which either holds a single clearly defined element or the list is empty. \$\endgroup\$ – chaotic3quilibrium Jun 6 '12 at 13:02
  • \$\begingroup\$ I read through the Java8 version of Option to which you linked. Ugh! Long before Java 8 is prevalent, I will be doing Scala full time on my projects. \$\endgroup\$ – chaotic3quilibrium Jun 6 '12 at 13:40
1
\$\begingroup\$

Your code essentially does one thing:

It replaces a NullPointerException with an UnsupportedOperationException (and even that only in some cases).

This is simply not a useful semantic.

It’s telling that this simple switch of one exception for another requires, in your code,

  • an interface
  • a static final helper class
  • a kind-of singleton (which isn’t really one)
  • a metric ton of unit tests that only test trivial behaviour.

Not to be blunt, but this code exemplifies everything that you can do wrong when going through the motions in Java. It uses unit tests, design patterns and follows non-fitting interface conventions (Bean-style getters even though your class is clearly not a bean) and achieves … essentially nothing.

Furthermore, this code is quite inefficient since it is implemented in terms of Set, and while that’s certainly very elegant, it results in tremendous runtime performance bloat.

In fact, due to its implementation in terms of Set the whole None special case is redundant since the Set already has this case built in – as an empty set.

Eventually this whole boilerplate code hides a single useful method1, namely getOrElse – and that method (which should be called just orElse) could just as easily be a static method in some helper class. And I’m not convinced that writing obj.orElse(other) is so much more readable than obj != null ? obj : other that it requires its own method.


1 The iterator interface is actually also potentially useful but also doesn’t justify this amount of boilerplate.

\$\endgroup\$
  • \$\begingroup\$ I do not completely agree with you. The exception replacement is true. I would prefer a simple orElse method. But one other thing that this code is contributing to is communication. It tells the user of the class what to expect from the method. It is impossible to tell if a function can return null, while an Option is clearly communicating that you cannot depend on getting what you are requesting from that method. \$\endgroup\$ – daramarak Jun 8 '12 at 10:26
  • 1
    \$\begingroup\$ @daramarak Unfortunately, due to Java’s semantics, a null value is always to expected, unless documented otherwise. So I really don’t think using Option adds anything. The opposite would, i.e. some class Value (or NonNull or similar) that guarantees a non-null value. And yes, such a class is actually a useful tool. \$\endgroup\$ – Konrad Rudolph Jun 8 '12 at 10:29
  • \$\begingroup\$ True, but with this class it is possible to write a library or application simply state that all methods will return a value unless actually returning a Option. But I feel this is more a weakness to the language, it should support option rather than supporting null (if it could be implemented cost free of course :D) \$\endgroup\$ – daramarak Jun 8 '12 at 10:46
  • \$\begingroup\$ @KonradRudolph Your response is pretty strong. I disagree that it only replaces one thing for another. After having used it in code I was writing just today, there were several instances where it substantially eliminated a series of intricate constructors based on parameters which could be passed null. Using Option, I was able to drop it to a single constructor and then used Option as my means to direct defaults, all with substantially less null checking boilerplate than I had there before. It also delightfully simplified understanding the parameters to the constructor. \$\endgroup\$ – chaotic3quilibrium Jun 8 '12 at 23:16
  • 1
    \$\begingroup\$ @chaotic3quilibrium If you can’t deal with criticism I’d rather suggest you don’t post on a code review site. \$\endgroup\$ – Konrad Rudolph Jun 10 '12 at 8:13
1
\$\begingroup\$

Just a few notes about the tests:

Too many asserts in one test is a bad smell. It's Assertion Roulette and you lost Defect Localization. If the first assert method throws an exception you won't know anything about the results of the other assert calls which could be important because they could help debugging and defect localization.

Furthermore, adding a message parameter to every assert is a good practice and helps debugging. Usually you get the line number in a stack trace when a test fails but a unique message makes debugging faster. Small unique numbers or short unique strings are usually enough if you have more than one assert in a test method.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.