Github user dsmiley commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/384#discussion_r191574085
--- Diff:
lucene/analysis/common/src/java/org/apache/lucene/analysis/miscellaneous/ConcatenateGraphFilter.java
---
@@ -119,27 +147,19 @@ public boolean incrementToken() throws IOException {
@Override
public void end() throws IOException {
- super.end();
- if (finiteStrings == null) {
- inputTokenStream.end();
- }
- }
-
- @Override
- public void close() throws IOException {
- if (finiteStrings == null) {
- inputTokenStream.close();
- }
+ restoreState(endState);
}
+ //nocommit move method to before incrementToken
@Override
public void reset() throws IOException {
- super.reset();
- if (hasAttribute(CharTermAttribute.class)) {
- // we only create this if we really need it to safe the UTF-8 to
UTF-16 conversion
- charTermAttribute = getAttribute(CharTermAttribute.class);
--- End diff --
Oh *that* comment. By that I mean move the _method_ to follow a natural
sequence. I like classes to have methods that are organized sensibly. When
one uses a TokenStream, reset() should come before incrementToken() ! -- then
end(), then close(), and any utility methods should come after callers.
I moved the initialization to reset simply because it felt natural this way
rather than incrementToken where it's lazy. That's all. That said... I hinted
at a TestRandomChains failure about lifecycle needing to throw an
IllegalStateException or something like that (pertaining to missing close()),
and I think making this back to lazy will "fix" that.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
