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:
- Option - public interface
- Optional - factory for safely producing instances of Some and None
- 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"));
}
}
4 Answers 4
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.
-
\$\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– chaotic3quilibrium2012年06月06日 13:12:39 +00:00Commented Jun 6, 2012 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– chaotic3quilibrium2012年06月06日 13:22:37 +00:00Commented Jun 6, 2012 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– WeaponsGrade2012年06月06日 14:41:17 +00:00Commented Jun 6, 2012 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 implementequals()
andhashCode()
,equals()
almost being an alias formatch()
(match is type-safe, whereas equals cannot be). So part of the reasoning was type safety...the compiler won't let you callOption<A>.match(Option<B>)
. \$\endgroup\$WeaponsGrade– WeaponsGrade2012年06月06日 14:45:49 +00:00Commented Jun 6, 2012 at 14:45 -
1\$\begingroup\$ There, added
equals
andhashCode
, as well as a missing factory methodOption
which will take anA
and turn it into aNone
if null, orSome(a)
otherwise. \$\endgroup\$WeaponsGrade– WeaponsGrade2012年06月06日 15:01:36 +00:00Commented Jun 6, 2012 at 15:01
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)
-
\$\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– chaotic3quilibrium2012年06月06日 13:02:25 +00:00Commented Jun 6, 2012 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– chaotic3quilibrium2012年06月06日 13:40:22 +00:00Commented Jun 6, 2012 at 13:40
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.
-
\$\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– daramarak2012年06月08日 10:26:54 +00:00Commented Jun 8, 2012 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 classValue
(orNonNull
or similar) that guarantees a non-null value. And yes, such a class is actually a useful tool. \$\endgroup\$Konrad Rudolph– Konrad Rudolph2012年06月08日 10:29:26 +00:00Commented Jun 8, 2012 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– daramarak2012年06月08日 10:46:07 +00:00Commented Jun 8, 2012 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– chaotic3quilibrium2012年06月08日 23:16:38 +00:00Commented Jun 8, 2012 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– Konrad Rudolph2012年06月10日 08:13:37 +00:00Commented Jun 10, 2012 at 8:13
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.
Option
implementation. \$\endgroup\$Optional<T>
. What we need even more is something similar to Scala'sTry[T]
. \$\endgroup\$