[
https://issues.apache.org/jira/browse/ACCUMULO-675?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13410467#comment-13410467
]
Billie Rinaldi commented on ACCUMULO-675:
-----------------------------------------
I'm in favor of the javadocs explaining how the usage of WrappingIterator has
changed. If subclasses are allowed to change seenSeek, I could see people
getting an error when they do something like findTop() in the init method, and
just setting seenSeek to true to get rid of the error without learning about
the iterator lifecycle. Perhaps this is misguided, but I think at this stage
we want to draw as much attention as possible to the fact that we're trying to
define the iterator lifecycle more clearly. I could see backing this off at
some point once people have become familiar with the lifecycle. We could even
consider removing these checks from WrappingIterator eventually, although
they've been useful for making sure our internal iterators are doing the right
things.
What do you think about the following?
{noformat}
/**
* A convenience class for implementing iterators that select, but do not
modify, entries read from a source iterator. Default implementations exist for
all
* methods, but {@link #deepCopy} will throw an
<code>UnsupportedOperationException</code>.
*
* This iterator has some checks in place to enforce the iterator contract.
Specifically, it verifies that it has a source iterator and that {@link #seek}
has
* been called before any data is read. If either of these conditions does not
hold true, an <code>IllegalStateException</code> will be thrown. In particular,
* this means that <code>getSource().seek</code> and <code>super.seek</code> no
longer perform identical actions. Implementors should take note of this and if
* <code>seek</code> is overridden, ensure that <code>super.seek</code> is
called before data is read.
*/
{noformat}
> WrappingIterator's seenSeek should be protected
> -----------------------------------------------
>
> Key: ACCUMULO-675
> URL: https://issues.apache.org/jira/browse/ACCUMULO-675
> Project: Accumulo
> Issue Type: Bug
> Components: tserver
> Affects Versions: 1.5.0, 1.4.1, 1.5.0-SNAPSHOT, 1.4.0
> Environment: OSX, Linux
> Reporter: William Slacum
> Assignee: William Slacum
> Priority: Trivial
> Fix For: 1.5.0-SNAPSHOT
>
> Attachments: wrapping-iterator-mod.patch
>
> Original Estimate: 1m
> Remaining Estimate: 1m
>
> In 1.3, the WrappingIterator was pretty much some boilerplate code. In 1.4
> on, a package private boolean called seenSeek was added to help enforce the
> iterator contract.
> This causes some issues with iterators written for 1.3 and before, because
> the seenSeek property can't be set by an iterator outside of the
> core.iterators package, which is locked down. This means that sub iterators
> must always delegate up to the WrappingIterator's seek() method, even if
> implementors want to completely override seek().
> I would like to provide more documentation on the WrappingIterator and make
> the seenSeek property protected so implementors don't need conditional logic
> to make the call to super.seek().
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
