Response to the critique for my last post and OneElementIterator

Much faster to avoid the try (which as you implied, can be quite slow) and instead use a local temporary:

public T next() {
  T e = element;
  element = null;
  return e;
}

It’s not concurrency safe, but then, neither is yours. :-)

Yes, thought about that, but was intrigued by the beauty of the finally solution. In production I would introduce the e reference though.

But I’m not sure about performance, because there is no Exception being thrown or catched.

According to the Iterator class javadoc, next() method is supposed to throw NoSuchElementException when iteration has no more elements. Also, there is a remove() method, which should throw an UnsupportedOperationException if the remove operation is not supported by Iterator.

@Eugene: You’re right of course.

The remove got lost with cut&paste (see missing } for class :-)

I’ll add the method.

What if you want to return an iterator that contains a _null_ :-)

And of course you have the while(true) System.out.println(e.next()); which relies on the exception to be thrown. It’s not good code, but it is valid. Violating the javadoc requirement of throwing the exception would put some unlucky user into a tight loop…

I’d go with Collections.singletonList(whatever).iterator(). The less code you write, the fewer bugs you write.

@Erik: The whole point was not to return null ;-)

“@Eugene: You’re right of course.”

So I added to my version

  public T next() {
    if (!hasNext()) {
      throw new NoSuchElementException("No more elements in OneElementIterator");
    }
    try {
      return element;
    } finally {
      element = null;
    }
  }

“I’d go with Collections.singletonList(whatever).iterator(). The less code you write, the fewer bugs you write.”

Normally I would agree. There are some posts on this topic on this blog, for example the String.reverse issue (JDK versus write-your-own).

In this case, because Option[T] is potentially used a lot, I’d go for an optimization.

Stephan, it seem to me that you created a problem that does not actually exist. Basically, if you are always using iterator, just for the sake of syntactic sugar with the foreach loop, according to my microbenchmark you got yourself about 60% overhead (even with your optimized OneItemIterator class) comparing to simply returning a Null object. It is actually unlikely something like that ever going to be a bottle neck in the real application, but number of iterations and tiny mistakes you made while implementing such simple class is really scare. So, it would have been safer to simply use existing JRE classes.

Nice :)

I guess we could rename OneElementIterator to Some, extend Option[T] and return this in iterator() to realign with the last post concepts.

@Eugene: I’m not sure what’s more important, less bugs or performance. You decide. Otherwise I fear there is only Scala left for me.

@lqd: Hmm :-)

If you don’t care about performance, Collections.singleton(value).iterator. :-)

If you want performance and null handling, add a mutable “hasNext” boolean flag to your implementation.

It’s important to mention that, unlike Java style external iterators, the Scala solution based on map and flatMap or the Haskell solution based on bind and “return” (not the same thing as Java style return) require no mutation. E.g., here’s a complete Option monad in Scala – it’s even covariant, which is a pain in the arse in Java

sealed abstract class MyOption[ A] {
def map[B](f:A=>B):MyOption[B] = flatMap(x => MySome(f(x)))
def flatMap[B](f:A=>MyOption[B]):MyOption[B] = this match {
case MySome(x) => f(x)
case _ => MyNone
}
}
case class MySome[ A](value:A) extends MyOption[A]
case object MyNone extends MyOption[Nothing]

println(for {x <- MySome(3);y <- MySome(4);} yield x y)

The Haskell option monad is even tighter with less syntactic noise (although, I know this will get totally borked by word press, I hope something makes it through)

data MyOption a = MySome a | MyNone deriving Show

instance Monad MyOption where
(MySome x) >>= k = k x
MyNone >>= k = MyNone
return = MySome
fail s = MyNone

main = print $ do x <- MySome 3; y <- MySome 4; return (x y)