Shalom Shai, Am 2012-11-04 20:25, schrieb Shai Erera:
Hey Mike,I'm not sure that I like the idea of throwing LuceneException or SearchException everywhere. I've been there (long time ago) and I always hated it.
Not everywhere but in the top-level API would be sufficient. Some code is never used/should be directly by the client. What is the reason for why you dislike such an approach?
First, what's the difference between 'new SearchException("Failed to read
the index", ioe)' and 'new IOException("Failed to read the index",
anotherIOE/anyE)'? Both don't give you any real way of detecting the
problem in your code, maybe only in the logs.
Perhaps you would like us to throw better IOE, i.e. rather than re-throwing
low-level IO exceptions, wrap them w/ another IOE w/ a detailed message.
That I can support, but do you have a concrete case where the exception
that was thrown was not descriptive enough?
There is a tremendous difference. I have possibility at compile time to detect whether an IOE came from Lucene or some other IO related action.
Consider this code:
try {
File mike = new File(...);
Reader r = new InputStreamReader(new FIS(mike)):
searcher.search(...);
}
catch (IOE e) {
}
Now, how am I supposed to figure out at compile time where this IOE came
from? I cannot neither react properly nor present a suitable user
response unless I do create for every IOE piece a separate try-catch-block.
This is just bloat. I cannot even log correctly because I do not know
what the source of the IOE was.
What am I excepted to do when every method throws just new Exception()? Absolutely nothing they would all look alike at compile time.
For instance, there's that "read past EOF" exception which is annoying b/c you can tell by the stacktrace where it happened, but not necessarily why it happened. However, these are fairly low in our call stack, and I doubt that even Lucene code itself can give meaningful messages to all low-level IOEs. Yet, the exception does tell you that's usually an index corruption case, or some other bug ... when Lucene detects the index is corrupt, it throws the explicit CorruptIndexEx, otherwise, this must be inferred from the stacktrace. Not perfect, but throwing IndexingException won't improve it either.
It would cover at least the case described above.
So I'm +1 for making sure our exceptions are as informative as they can get. I'm -1 for starting to throw LuceneException all over the place - to me it's like IOE. Worse, if we start throwing LuceneException, people might be tempted to wrap any Ex w/ LuceneEx, even ArrayIndexOutOfBound etc. I don't want that.
That should not be the case. IndexOutOfBoundsException is an unchecked exception. There is a huge difference to IOE which is a checked exception. Joshua Bloch exactly describes the case of the IOOBE in item 58. IIOOBE must remain as unchecked because it indicates a violation of the API contract. E.g., a corrupt index is recoverable and not a programming error. This can happen through various reasons you cannot control.
If you've hit exceptions which could use better messages, I prefer that we concentrate on them, rather than complicating the exceptions throwing mechanism.
I definitively will. As I always do with OSS.
On Sun, Nov 4, 2012 at 7:25 PM, Michael-O <1983-01...@gmx.net> wrote:Hi Simon, there are generally two very good resources how exceptions should be handled in Java. I will quote both: 1. Oracle's JavaDoc Style Guide: http://www.oracle.com/** technetwork/java/javase/**documentation/index-137868.**html#throwstag<http://www.oracle.com/technetwork/java/javase/documentation/index-137868.html#throwstag> 2. Joshua Bloch's book Effective Java, chapter 9: http://my.safaribooksonline.**com/book/programming/java/**9780137150021<http://my.safaribooksonline.com/book/programming/java/9780137150021> Am 2012-11-02 22:27, schrieb Simon Willnauer: Hey,On Fri, Nov 2, 2012 at 2:20 PM, Michael-O <1983-01...@gmx.net> wrote:Hi, why does virtually every method (exaggerating) throw an IOE? I know there might be a failure in the underlying IO (corrupt files, passing checked exc up, etc) but 1. Almost none of the has a JavaDoc on itwhat should the javadoc say? I mean it would repeat itself all the time no?At least one would know why this one is thrown. See source one. 2. Throwing an IOE from most of the methods doesn't really help.You cannot create separate catch blocks. You have to rip your code apart in tens of try catch blocks. Here is a simple example: Both IndexSearcher#search and IndexSearcher#doc throw an IOE with any further documentation. I don't have the chance to detect where the exception has happened nor I can pass something reasonable back to the user. And both methods are keypoints in Lucene.can you elaborate what you would change to make this easier digestible for you? I mean specialized exceptions class would be a mess here really.Not only for me but for everyone. Generally, good exception handling should setup a simple exception hierarchy, not more than three levels. Two base classes for checked and unchecked exceptions. A main exception for a logical group like searching, retreival, query checking, etc. LuceneException |-- SearchException |-- RetrievalException |-- ... One should really wrap IOE in something like 'new SearchException("Failed to read the index", ioe)'. I would at least know that this one came from the index searcher. Really advisable are items 61, 62 and especially 63. E.g., there are exceptions thrown in IndexSearcher without anymessage. Simply empty stubs.I agree we should fix them. I already committed some fixes to IndexSearcher. Can you come up with a patch that fixes some more in core? running grep -R "new.*Exception()" * | wc -l yields 82 in core so there is room for improvement.Lucene 4.0 received a complete API overhaul. Why was no action taken to clean up exception management?I'd really like to hear what you have in mind. can you elaborate?Continuing my answer from above. Have you ever worked with the Spring Framework? They apply a very nice exception translation pattern. All internal exceptions are turned to specialized unchecked exceptions like AuthenticationExceptoin, DaoAccessException, etc. Lucene has already good exceptions like CorruptIndex, IndexNotFound, LockObtainFailed, etc. Of course, such a step would break backwards-compat but this is possible for 5.0 and this would require a general consensus of the devs improving this issue. I think that such a great framework can do better to inform programmers and users what has really failed especially if you design a public API. We have succeeded exceptional results with the Lucene framework. I am someone who likes giving help back to OSS projects. Hopefully, this is enough input for this message. We could discuss this further is there is a real interest from the devs. Mike ------------------------------**------------------------------**--------- To unsubscribe, e-mail: java-user-unsubscribe@lucene.**apache.org<java-user-unsubscr...@lucene.apache.org> For additional commands, e-mail: java-user-help@lucene.apache.**org<java-user-h...@lucene.apache.org>
--------------------------------------------------------------------- To unsubscribe, e-mail: java-user-unsubscr...@lucene.apache.org For additional commands, e-mail: java-user-h...@lucene.apache.org
