On 13/01/2012 8:50 AM, Dmitry Samersoff wrote:
On 2012-01-13 02:30, Kurchi Hazra wrote:
255-258 a) Exceptions could be collapsed
This cannot be collapsed since FileNotFoundException is a subclass of
IOException.
Does it mean that we can simple remove catch of FileNotFoundException,
replacing it with a comments that FileNotFound case covered by IOException ?
263 } catch (FileNotFoundException e) {
264 error(CONFIG_FILE_OPEN_FAILED, e.getMessage());
265 } catch (IOException e) {
266 error(CONFIG_FILE_OPEN_FAILED, e.getMessage());
This code looks really weird for me, could you at least add a comment?
It is redundant to catch FileNotFoundException and IOException and
perform the same action in each case. This can simply reduce to catching
IOException.
b) finally is gone - is it expected?
CONFIG_FILE_CLOSE_FAILED is never happens anymore.
I would prefer to keep original code
- We were trying to use try-with-resources (new JDK7 feature), where
closing of resources is automatically taken care of. But I will
revert back to original code if that is preferred.
We going to an area of changing public symbols. I'm in doubt that
someone rely on CONFIG_FILE_CLOSE_FAILED message, but we need to check
it, remove it from all translations etc.
IMHO, It's out of the scope of your amazing work.
I agree. Changing to try-with-resources will change the failure mode.
I'm not 100% sure how the try-with-resources works in this case but it
seems to me that an IOException on the close() would result in a call to
error() with the CONFIG_FILE_OPEN_FAILED message.
David
-----
