views:

307

answers:

7

I've heard people advise that one should always use the Iterator pattern to control loops rather than throwing an exception (which is how it's done in Python iterators) or using the Sentinel pattern, whereby a special, sentinel value (often null) is returned to indicate the end of the iteration.

Does best practice advise against the sentinel pattern? If so, why? (other than it not working with the foreach syntax in Java 1.5).

Edit: Code example 1 - Sentinel Pattern

Reader r = ...;
for( int val = r.read(); val != -1; val = r.read()) {
   doSomethingWith(val);
}

Code example 2 - Iterator Pattern

for(Iterator<Thing> it = getAnIterator() ; it.hasNext(); ) {
  Thing t = it.next();
  doSomethingWith(t);
}
+15  A: 

The issue with the Sentinel pattern is that it explicitly excludes the sentinel value from the set of values that are valid elements. I.e., if you have a list of objects which can validly contain null as an element, using null as a sentinel value is a fail.

McWafflestix
+9  A: 

Exceptions should only be used in "exceptional" situations. Concluding or breaking out of a loop is normal program flow, not an exceptional situation.

Additionally, when working in Java, (or any language for that matter), you want to use patterns and conventions that are common and well known in the community because other Java programmers may need to maintain your code. If they do, they will most likely expect to see Iterators, not the sentinel pattern.

Asaph
I agree with this principle and recommend following it. The dirty secret is that the exception throw version is insanely fast, esp if you used a cached exception singleton. But I wouldn't do that unless I needed insane speed and I could prove it was faster. I mention it here merely because it's interesting.
Alex Miller
A: 

I don't see how the Sentinel Pattern is different enough to be its own pattern. My only thought is that it would be useful for determining a stop value from streaming data. However, the interator takes care of this with the "hasMore" method requirement.

monksy
+2  A: 

The Sentinel pattern is used in the JDK for reading streams, so it isn't unheard of, but it is awkward to make a loop out of. All solutions have negatives that an iterator doesn't. Your solution requires a repetitive call to read (that is code duplication). A while loop forces the variable to be declared outside the scope of the loop. Other alternatives would be surprising and hard to follow.

So in the end the iterator is kind of the default that should only be deviated from for a reason.

Yishai
+4  A: 

I think they're both ok but the Iterator is more idiomatic in Java (particularly if you actually have an Iterable that you can use the for-each loop on instead).

The one place you do see the Sentinel version a lot in Java is exactly the case you've written in I/O code.

Alex Miller
+1 for pointing out that there is no hard and fast rule that covers all cases, each pattern has its own pros and cons.
MAK
+2  A: 

The mantra is "say what you do, and do what you say."

If you test the returned value for being a special value, that says nothing about why you test this. In your example:

for( int val = r.read(); val != -1; val = r.read()) {
   doSomethingWith(val);
}

Does this say "if the returned value ever becomes -1, we can skip the rest", or "if the returned value ever is -1, an error has occurred", or "if the returned value ever is -1, the end is reached"? In contrast, hasNext is completely unambiguous.

By the way, I actually like the foreach and map constructs other languages provide (or allow to write) better than explicit looping.

Svante
A: 

"Exceptions should only be used in "exceptional" situations. Concluding or breaking out of a loop is normal program flow, not an exceptional situation."

If a file read succeeds a million times and only the last time it says "I can't find any more records", you don't call that "exceptional" ?

I don't see the problem in using, say, EOFExceptions for control flow (if the System is sane enough to raise them, instead of returning that awfully stupid -1 integer saying that "-1 bytes were read"). You tell me if the following is unreadable/unmaintainable :

try {
    r = readNext(...);
    process(r);
} catch (EOFException e) {
    ...
}
Erwin Smout
If you don't like the Sentinel, a boolean method is much better than just catching an exception out of a while(true) block.
Yishai