[
https://issues.apache.org/jira/browse/LUCENE-818?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12478327
]
Michael McCandless commented on LUCENE-818:
-------------------------------------------
> 2) the body of the catch clauses could be put into a helper method just like
> ensureOpen to help reduce code noise
True, but I think it's still more code noise to have
try/catch/call-helper-method than ensureOpen() call? Esp. since you'd
have to handle IOException and RuntimeException as 2 separate catch
clauses (I think?), each of which calls a separate helper. Ie instead
of:
ensureOpen();
...do stuff...
we would need something like:
try {
...do stuff...
} catch (IOException exc) {
throw ensureOpenAfterIOException(exc);
} catch (RuntimeException exc) {
throw ensureOpenAfterRuntimeException(exc);
}
I think?
> 3) if there are situations where damage will be done by not testing that we
> are open before taking some action, that would fall under my "adding better
> error checking in those specific cases (if we know of any) and throwing
> explicit exceptions." ... a lot of this could be achieved (as Yonik
> suggested) by nulling out more things in close so that the first attempt to
> do something dangerous after the close triggered a NullPointerException.
The thing is we may not know all such cases (yet)? I prefer taking a
defensive approach here. I don't really like the null-out solution
because I think getting an AlreadyClosedException is clearer to the
user than a NPE's.
> 4) "fail fast" is always good ... except when it makes the non-failure case
> slow ... i was merely suggesting an alternative that would achieve the same
> results without penalizing performance of people obeying the rules.
>
> if numDoc(), maxDoc(), isDeleted(), and hasDeletions() are the only mehtods
> were people are concerned about the performance impacts of calling
> ensureOpen() everytime, then those methods could be the ones where isOpen
> could be checked in any exception handling block, and all of the other
> mehtods could use ensureOpen as orriginal described.
Good idea! An added bonus is that these methods do not throw
IOException so the exception handling would just have the one
RuntimeException catch clause above. OK I will rework patch with this
approach and see how it looks...
> IndexWriter should detect when it's used after being closed
> -----------------------------------------------------------
>
> Key: LUCENE-818
> URL: https://issues.apache.org/jira/browse/LUCENE-818
> Project: Lucene - Java
> Issue Type: Bug
> Components: Index
> Affects Versions: 2.1
> Reporter: Michael McCandless
> Assigned To: Michael McCandless
> Priority: Minor
> Attachments: LUCENE-818.patch, LUCENE-818.take2.patch,
> LUCENE-818.take3.patch
>
>
> Spinoff from this thread on java-user:
> http://www.gossamer-threads.com/lists/lucene/java-user/45986
> If you call addDocument on IndexWriter after it's closed you'll hit a
> hard-to-explain NullPointerException (because the RAMDirectory was
> closed). Before 2.1, apparently you won't hit any exception and the
> IndexWrite will keep running but will have released it's write lock (I
> think).
> I plan to fix IndexWriter methods to throw an IllegalStateException if
> it has been closed.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
