Eddie Epstein wrote:
On Wed, Mar 19, 2008 at 9:28 AM, Adam Lally (JIRA)
<[email protected]> wrote:
Users sometimes need to implement the Java iterator interface, in which case
they need to make sure that their hasNext method doesn't advance the iterator -
so I don't see that this is that different. I guess a difference in UIMA's
case is that there's another level of indirection (the CasIterator) between the
calling code and the CasMultiplier, so we could afford to have different
contracts at the two layers. CasIterator's hasNext could be called any number
of times, but the CasMultiplier's would be guaranteed to only get called once.
I think that's what your fix is doing. So since end users never interact with
the CasMultiplier interface directly, it's not a huge loss if it's contract
doesn't follow the standard iterator paradigm.
I guess I still have a preference for keeping the contract with the
CasMultiplier analogous to the normal Iterator contract, since I think most
people think that an interface with hasNext/next operates in this way. And who
knows, maybe someone else has written some code that directly calls a
CasMultiplier component outside of the UIMA framework, and they'd be tripped up
by this. Unlikely, but possible.
Does anyone else want to voice an opinion?
+1, agree completely.
The documentation claiming that the work in CAS multipliers can be
distributed between hasNext and next is currently insufficient. Either
it should be changed to warn users to protect their hasNext code from
being called multiple times, or the UIMA code changed to avoid hasNext
being called unnecessarily. I'd vote for changing the code instead of
the documentation.
As Adam pointed out, the code that implements the Iterator interface
should follow the Iterator contract. You're allowed to call hasNext()
any number of times before actually advancing the iterator. Whether
the actual work of advancing the iterator happens in the next() or
hasNext() method is immaterial. I have written Iterator implementations
like this:
public Object next() {
if (hasNext()) {
return next;
}
throw new NoSuchElementException();
}
where the actual work happens in hasNext(). The user may or may not
call hasNext(), doesn't matter. Of course in the implementation of
hasNext(), I have to make sure to remember that I have advanced the
iterator. Sometimes it works better that way, for internal reasons.
I have read the documentation again, and I think it's fine. The
bit about distributing the processing makes perfect sense to me.
So does the part about the calling sequence, but then it wouldn't
occur to me to write an iterator whose hasNext() method may only be
called once before next() is called. If you think that's necessary,
we could add a note to indicate that hasNext() may be called more
than once.
--Thilo
User code that directly calls a CasMultiplier component outside of the
UIMA framework should get exactly what it asks for.
Eddie
