Re: IndexReader.indexExists declares throwing IOE, but never does
Technically, there's a big difference between I checked, and there was no index, and I was unable to check the disk because file system went BANG!. So the proper behaviour is to return false IOE (on proper occasion)? On Mon, Mar 21, 2011 at 13:53, Michael McCandless luc...@mikemccandless.com wrote: On Mon, Mar 21, 2011 at 12:52 AM, Shai Erera ser...@gmail.com wrote: Can we remove the declaration? The method never throws IOE, but instead catches it and returns false. I think it's reasonable that such a method will not throw exceptions. +1 -- Mike http://blog.mikemccandless.com - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org -- Kirill Zakharenko/Кирилл Захаренко E-Mail/Jabber: ear...@gmail.com Phone: +7 (495) 683-567-4 ICQ: 104465785 - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
Re: IndexReader.indexExists declares throwing IOE, but never does
2011/3/21 Earwin Burrfoot ear...@gmail.com: Technically, there's a big difference between I checked, and there was no index, and I was unable to check the disk because file system went BANG!. So the proper behaviour is to return false IOE (on proper occasion)? +1 to throw the exception when proper to do so Otherwise please keep the throws declaration so that you won't break public APIs if this changes implementation. On Mon, Mar 21, 2011 at 13:53, Michael McCandless luc...@mikemccandless.com wrote: On Mon, Mar 21, 2011 at 12:52 AM, Shai Erera ser...@gmail.com wrote: Can we remove the declaration? The method never throws IOE, but instead catches it and returns false. I think it's reasonable that such a method will not throw exceptions. +1 -- Mike http://blog.mikemccandless.com - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org -- Kirill Zakharenko/Кирилл Захаренко E-Mail/Jabber: ear...@gmail.com Phone: +7 (495) 683-567-4 ICQ: 104465785 - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
Re: IndexReader.indexExists declares throwing IOE, but never does
So the proper behaviour is to return false IOE (on proper occasion)? I don't object to it, as I think it's reasonable (as today we may be hiding some info from the app). However, given that today we never throw IOE, and that if we start doing so, we'll change runtime behavior, I lean towards keeping the method simple and remove the throws declaration. Well, it's either we change the impl to throw IOE, or remove the declaration altogether. Changing the impl to throw IOE on proper occasion might be problematic -- IndexNotFoundException is thrown when an empty index directory was given, however by its Javadocs, it can also indicate the index is corrupted. Perhaps the jdocs are wrong and it's thrown only if the index directory is empty, or no segments files are found. If that's the case, then we should change its javadocs. Otherwise, it will be difficult to know whether the INFE indicates an empty directory, for which you'll want to return false, or a corrupt index, for which you'll want to throw the exception. Besides, I consider this method almost like File.exists() which doesn't throw an exception. If indexExists() returns false, the app can decide to investigate further by trying to open IndexReader or read the SegmentInfos. But the API as-is needs to be simple IMO. Otherwise please keep the throws declaration so that you won't break public APIs if this changes implementation. Removing the throws declaration doesn't break apps. In the worse case, they'll have a catch block which is redundant? Shai On Mon, Mar 21, 2011 at 4:12 PM, Sanne Grinovero sanne.grinov...@gmail.comwrote: 2011/3/21 Earwin Burrfoot ear...@gmail.com: Technically, there's a big difference between I checked, and there was no index, and I was unable to check the disk because file system went BANG!. So the proper behaviour is to return false IOE (on proper occasion)? +1 to throw the exception when proper to do so Otherwise please keep the throws declaration so that you won't break public APIs if this changes implementation. On Mon, Mar 21, 2011 at 13:53, Michael McCandless luc...@mikemccandless.com wrote: On Mon, Mar 21, 2011 at 12:52 AM, Shai Erera ser...@gmail.com wrote: Can we remove the declaration? The method never throws IOE, but instead catches it and returns false. I think it's reasonable that such a method will not throw exceptions. +1 -- Mike http://blog.mikemccandless.com - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org -- Kirill Zakharenko/Кирилл Захаренко E-Mail/Jabber: ear...@gmail.com Phone: +7 (495) 683-567-4 ICQ: 104465785 - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
Re: IndexReader.indexExists declares throwing IOE, but never does
2011/3/21 Shai Erera ser...@gmail.com: So the proper behaviour is to return false IOE (on proper occasion)? I don't object to it, as I think it's reasonable (as today we may be hiding some info from the app). However, given that today we never throw IOE, and that if we start doing so, we'll change runtime behavior, I lean towards keeping the method simple and remove the throws declaration. Well, it's either we change the impl to throw IOE, or remove the declaration altogether. Changing the impl to throw IOE on proper occasion might be problematic -- IndexNotFoundException is thrown when an empty index directory was given, however by its Javadocs, it can also indicate the index is corrupted. Perhaps the jdocs are wrong and it's thrown only if the index directory is empty, or no segments files are found. If that's the case, then we should change its javadocs. Otherwise, it will be difficult to know whether the INFE indicates an empty directory, for which you'll want to return false, or a corrupt index, for which you'll want to throw the exception. Besides, I consider this method almost like File.exists() which doesn't throw an exception. If indexExists() returns false, the app can decide to investigate further by trying to open IndexReader or read the SegmentInfos. But the API as-is needs to be simple IMO. good points, I withdraw my previous objection :) Otherwise please keep the throws declaration so that you won't break public APIs if this changes implementation. Removing the throws declaration doesn't break apps. In the worse case, they'll have a catch block which is redundant? yes you wouldn't make any harm now, but if you release it without, and then figure you actually need to add it back in future, people might have code which is not handling it. I'm looking into Lucene 3.0.3 and the IOException it *is* actually needed, not sure what was changed in the version this is referring to, but as it used to throw it (and needing it), I think it's quite possible this need is not so remote. Regards, Sanne Shai On Mon, Mar 21, 2011 at 4:12 PM, Sanne Grinovero sanne.grinov...@gmail.com wrote: 2011/3/21 Earwin Burrfoot ear...@gmail.com: Technically, there's a big difference between I checked, and there was no index, and I was unable to check the disk because file system went BANG!. So the proper behaviour is to return false IOE (on proper occasion)? +1 to throw the exception when proper to do so Otherwise please keep the throws declaration so that you won't break public APIs if this changes implementation. On Mon, Mar 21, 2011 at 13:53, Michael McCandless luc...@mikemccandless.com wrote: On Mon, Mar 21, 2011 at 12:52 AM, Shai Erera ser...@gmail.com wrote: Can we remove the declaration? The method never throws IOE, but instead catches it and returns false. I think it's reasonable that such a method will not throw exceptions. +1 -- Mike http://blog.mikemccandless.com - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org -- Kirill Zakharenko/Кирилл Захаренко E-Mail/Jabber: ear...@gmail.com Phone: +7 (495) 683-567-4 ICQ: 104465785 - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
Re: IndexReader.indexExists declares throwing IOE, but never does
2011/3/21 Shai Erera ser...@gmail.com: So the proper behaviour is to return false IOE (on proper occasion)? I don't object to it, as I think it's reasonable (as today we may be hiding some info from the app). However, given that today we never throw IOE, and that if we start doing so, we'll change runtime behavior, I lean towards keeping the method simple and remove the throws declaration. Well, it's either we change the impl to throw IOE, or remove the declaration altogether. Changing the impl to throw IOE on proper occasion might be problematic -- IndexNotFoundException is thrown when an empty index directory was given, however by its Javadocs, it can also indicate the index is corrupted. Perhaps the jdocs are wrong and it's thrown only if the index directory is empty, or no segments files are found. If that's the case, then we should change its javadocs. Otherwise, it will be difficult to know whether the INFE indicates an empty directory, for which you'll want to return false, or a corrupt index, for which you'll want to throw the exception. Besides, I consider this method almost like File.exists() which doesn't throw an exception. If indexExists() returns false, the app can decide to investigate further by trying to open IndexReader or read the SegmentInfos. But the API as-is needs to be simple IMO. File.exists() parallel is a good one. So, maybe, it's ok ) Otherwise please keep the throws declaration so that you won't break public APIs if this changes implementation. Removing the throws declaration doesn't break apps. In the worse case, they'll have a catch block which is redundant? Shai On Mon, Mar 21, 2011 at 4:12 PM, Sanne Grinovero sanne.grinov...@gmail.com wrote: 2011/3/21 Earwin Burrfoot ear...@gmail.com: Technically, there's a big difference between I checked, and there was no index, and I was unable to check the disk because file system went BANG!. So the proper behaviour is to return false IOE (on proper occasion)? +1 to throw the exception when proper to do so Otherwise please keep the throws declaration so that you won't break public APIs if this changes implementation. On Mon, Mar 21, 2011 at 13:53, Michael McCandless luc...@mikemccandless.com wrote: On Mon, Mar 21, 2011 at 12:52 AM, Shai Erera ser...@gmail.com wrote: Can we remove the declaration? The method never throws IOE, but instead catches it and returns false. I think it's reasonable that such a method will not throw exceptions. +1 -- Mike http://blog.mikemccandless.com - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org -- Kirill Zakharenko/Кирилл Захаренко E-Mail/Jabber: ear...@gmail.com Phone: +7 (495) 683-567-4 ICQ: 104465785 - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org -- Kirill Zakharenko/Кирилл Захаренко E-Mail/Jabber: ear...@gmail.com Phone: +7 (495) 683-567-4 ICQ: 104465785 - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
