finally block does not complete normally - Eclipse

[Mark 'Kamikaze' Hughes]

Yes. You should handle exceptions as close to their cause as you
reasonably can.

[...]
Even worse (as I explained later in my first reply), you are potentially
ignoring serious problems in your code and continuing as if the file
write operating succeeded.

Clearly Chris is having a hard time thinking clearly here. "Handling
an exception" doesn't mean throwing it away. The kind of "thinking"
that leads to that kind of conclusion is just amazing. And he claims to
be some kind of programmer?

Catching an exception without handling it is always wrong, whether you
catch it near or far. When you leave the "catch" block, your program
should continue in a desired direction, even when that is the exit.

Here's why you handle it locally, at least initially: What filename
just caused this IOException? For any exception, you need to provide
context to have meaningful error messages, which must be done as close
to the source of the exception as possible.

You need to catch the exception, add some useful information, and
either pass it to an error handler directly, or throw an
application-specific exception with the current exception as a cause.

Throwing an exception and letting some top-level handler catch it is
often just not an acceptable choice: in an AWT event thread, for
instance. Or in a multithreaded application, where the exception has to
reach a different thread's handler.

 

[Chris Smith]

Clearly Chris is having a hard time thinking clearly here. "Handling
an exception" doesn't mean throwing it away. The kind of "thinking"
that leads to that kind of conclusion is just amazing. And he claims to
be some kind of programmer?

I'll try to slow down a bit, then. If you catch the exception before
you are in a position to handle it, then you are left with the following
options:

1. Rethrow it or some other exception.
2. Abandon the exception handling try/catch control flow.
3. Duplicate the try/catch control flow using conditionals at each level
of the call stack.

Now, obviously there do exist good reasons to do so, such as if the
exception is recoverable in this context. However, the original post by
Antti Brax didn't say to catch the exception "if it's a good idea". The
post recommended always catching exceptions in that context. That
implies that there are some times when you don't know what to do with
the exception, but are catching it anyway. It's that recommendation
with which I disagreed.

Of course you can log exceptions and do other similar things in any
catch block you like. However, if you'd like the exception to affect
the control flow of a larger operation, then you need to keep it in
play. Writing that file isn't the end of the story, and the question is
what happens next.

Here's why you handle it locally, at least initially: What filename
just caused this IOException? For any exception, you need to provide
context to have meaningful error messages, which must be done as close
to the source of the exception as possible.

You need to catch the exception, add some useful information, and
either pass it to an error handler directly, or throw an
application-specific exception with the current exception as a cause.

If you have information to add, then yes. However, rethrowing an
application-specific exception is irrelevant here, as Antti, with whom I
was originally discussing this, was talking about preventing the finally
block from completing abruptly.

Throwing an exception and letting some top-level handler catch it is
often just not an acceptable choice: in an AWT event thread, for
instance. Or in a multithreaded application, where the exception has to
reach a different thread's handler.

Quite the contrary, it is of course very possible to catch exceptions in
the AWT event thread, in multithreaded environments, etc. Sometimes
it's less obvious how to go about it, but I haven't seen a situation
where it's entirely impossible.

Frankly, I don't care what you have to say. You've proven yourself to
be an ass, and I don't care to continue the conversation. Bye.

--
www.designacourse.com
The Easiest Way To Train Anyone... Anywhere.

Chris Smith - Lead Software Developer/Technical Trainer
MindIQ Corporation

 

[Lasse Reichstein Nielsen]

I'm not sure I agree. Exceptions should, of course, be caught in the
"right" place, by which I mean the place that best balances various
development concerns. Sometimes that place is closer to the middle (for
example, five methods up, but not the seventeen methods all the way to
the root of the call tree.)

If "five methods up" is still within the same logical layer of your
application, the same layer of abstraction, then letting the exception
live until can make perfect sense. That is still near enough for me to
call it near It's when exceptions survive to a different level of
abstraction, where there is no chance that the receiver even understands
it, that something should be done.

In that case, you catch the exception, wrap it, and rethrow. That
doesn't really count as catching for the purposes of this conversation.

Indeed, it doesn't seem like it matches any of the cases below
.... except that it is exactly how I would implement number one (which
is therefore also the one that I would pick).

So, back in context again, the question is what to do with an exception
that is thrown during an OutputStream.close() operation.

Nasty place, indeed. And

The options were:

1. Don't catch it, and let it be handled elsewhere, realizing that this
could mask another I/O exception that was thrown earlier.

Since the exception thrown by OutputStream.close() is probably an
IOException, a checked exception, simply passing it outwards will
leave "throws IOException" all over the code upwards from there.

Catching the IOException and throwing a more meaningfull exception,
one that can be understood by the code receiving it, is not only
prettier, it also avoids exposing an implementation detail in the
signature of your method.

If there is no reasonable way for the calling code to handle an
exception at this point, it should perhaps be a runtime exception.
Runtime exceptions should generally not be caught (if they are thrown
appropriatly so that would mean the death of the program if not
caught by a very high level handler ... if such one makes sense.

2. Catch it and continue without reacting (perhaps some logging or other
reporting could be done, but it would have to be done from a catch
clause inside a finally block at the leaves of the call tree).

Proceeding should only be an option if the program knows it to be
safe, and that it is indeed the best cause of action. For this exact
exception, that is unlikely, but I bet there is at least one example
where it is.

3. Set a boolean flag of some sort, and write your own logic to replace
the normal throw/catch exception control flow.

Reinventing the wheel ... only square That would almost be back to
the C way of handling exceptional conditions: returning -1

I'm voting for #1. It leads to simpler code, less likelihood of missing
an important exception, and no substantial decrease in the likelihood of
having enough information to troubleshoot the original problem.

I agree, provided that it includes wrapping the exception when it
passes abstraction layers.

It doesn't really help for you to say that this should be done in a
"top-level handler", since the question is how to deal with issues
in a try/finally block, and we're not even dealing with how the
exception is handled from elsewhere in the call tree.

Whenever you throw an exception, you should be aware who, if any, will
catch it. Not the exact code, obviously, but what their role is in
calling you and what exceptions would make sense to them.

A top-level handler in an application is really a nobody. It has no
role related to you, so it's not really any better than having the
exception terminate the program. It's just the only case I can find
for not catching an exception close to where it is thrown (which
might include wrapping it and rethrowing).

/L

 

[Chris Smith]

If "five methods up" is still within the same logical layer of your
application, the same layer of abstraction, then letting the exception
live until can make perfect sense.

This is going to depend heavily on the architectural design, of course.
In general, I'd say that "same layer of abstraction" is a relative term.
There are progressively greater degrees of abstraction in a well-
designed piece of code. We pretty much agree, though. What matters is
the level of abstraction in which the exception itself makes sense. For
example, IOException makes sense when you know that you're doing I/O. A
few levels toward the root of the call tree, the I/O to the temporary
file becomes merely an implementation detail of how you build a PDF
report to deliver to the client, so you might convert it into a
ReportingException. Later on, the building of the report becomes an
implementation detail from the perspective of the application framework,
so it's wrapped again.

I just don't think there's any globally applicable definition for
"near" or "far" in that context, so I'd rather talk about the
appropriateness of the exception. I realize that you're saying the same
thing, but I didn't when I first responded.

Indeed, it doesn't seem like it matches any of the cases below
... except that it is exactly how I would implement number one (which
is therefore also the one that I would pick).

Yes, exactly. We definitely don't disagree on this.

Note that we don't have to cross an abstraction boundary for this to be
relevant. For example, I'm also saying that I prefer this:

try
{
OutputStream out = new FileOutputStream(fileName);

try
{
// write file
}
finally
{
out.close();
}
}
catch (IOException e)
{
throw new SubsystemException(e);
}

versus this:

OutputStream out = null;

try
{
out = new FileOutputStream(fileName);
// write file
}
catch (IOException e)
{
throw new SubsystemException(e);
}
finally
{
try
{
if (out != null) out.close();
}
catch (IOException e)
{
throw new SubsystemException(e);
}
}

No crossing of even a method boundary, but the issues are the same. The
first option is cleaner, but it allows the abnormal completion of a
finally clause.

Basically, the problem arises when a finally clause NEVER completes
normally, and that's the exact situation that Eclipse warns about.
Finally clauses that sometimes complete abnormally are fine. (When you
think about it, they are really inevitable given the existence of
java.lang.InternalError.)

--
www.designacourse.com
The Easiest Way To Train Anyone... Anywhere.

Chris Smith - Lead Software Developer/Technical Trainer
MindIQ Corporation

 

[Daniel Dyer]

When you're working with someone else and using a non-standard
indentation style, they're going to have to hit reformat in Eclipse, and
leave a giant chunk of diff in the CVS logs. This is your fault for
violating standards.

Nobody in the entire world cares about your personal artistic
expression through indentation style, they just want a common,
consistent style so they can get their work done.

That's not a problem if the whole project is standardised on Chris'
preferred indentation style. I used to use the Sun indentation style when
I started programming in Java, but every place I've worked has
standardised on the other way, the one that Chris prefers - and I have to
say that I now prefer this way too. The Sun style seems to be dominant in
academic institutions, but outside of that it seems a pretty even mix of
the two in my experience.

Dan.

 

[Chris Uppal]

I would suggest before you try what you call "joking" again, you learn
how to be funny. You were flaming with the light of a billion burning
nerds. Now that you've been called on it, you're just trying to
backpedal. That's "funny", in a sense, but only in the "we're all
laughing at you", not "we're all laughing with you" sense.

<plonk/>

-- chris

 

[Daniel Dyer]

That's not a problem if the whole project is standardised on Chris'
preferred indentation style. I used to use the Sun indentation style
when I started programming in Java, but every place I've worked has
standardised on the other way, the one that Chris prefers - and I have
to say that I now prefer this way too. The Sun style seems to be
dominant in academic institutions, but outside of that it seems a pretty
even mix of the two in my experience.

Replace "indentation" with "brace placement".

Dan.

 

[Antti S. Brax]

I strongly disagree that it's a good idea to put a try/catch inside the
finally.

After all, what do you intend to do with the exception when you catch
it? Are you saying that the code that reads the file ought to have
knowledge of the exception handling strategy for the software?

Hell yeah! It would be silly to demand that a software does not
know it's own exception handling strategy.

Too often, people taking this kind of advice -- to always handle
exceptions -- do something ineffective, like just write it to standard
output then drop it.

You are generalizing a very specific issue here (but I agree
with your generic advice anyway).

The problem is that if finally executes because
the original code completed normally, then you miss proper handling of
the exception and it very likely gets forgotten. That leads to
undetected faults in cases like running out of disk space while flushing
the buffers at the end of a write operation.

I see that you have a point and I agree partly. But how exactly
should the exception handling be done in the finally block so
that no important information is hidden from the caller? I came
up with a solution, but it is quite ugly and not at all perfect
(exception thrown in finally is still "hidden" if an exception
is thrown in try).

fos = new FileOutputStream(FILENAME);
boolean errors = true;
try {
// Manipulate output stream.
} finally {
try {
fos.close();
} catch (IOException ex) {
if (errors) {
// Log exception.
} else {
// Rethrow the exception.
}
}
}

Since it is likely (as you also say below) that whatever is
thrown in the close might be caused by the same error that
caused the earlier exception, it is more important to pass
the earlier exception to the caller.

Generally speaking,
though, the two exceptions will be caused by EXACTLY the same thing, and
having just one of the two exceptions for debugging is good enough.

No. The earlier exception is much more important, since it
also contains information about the state in which the (please
forgive me) "business logic" was when things went wrong. This
is usually quite important especially if fixing the problem
involves cleaning up things that were left in an invalid state.

 

[Chris Smith]

Hell yeah! It would be silly to demand that a software does not
know it's own exception handling strategy.

Perhaps it's silly, but that is exactly what exception handling is
designed to provide. By declaring an exception as a part of the public
interface of an API, you are allowing that error condition to occur,
while embedding absolutely no knowledge within the API itself about what
should be done in response.

I see that you have a point and I agree partly. But how exactly
should the exception handling be done in the finally block so
that no important information is hidden from the caller? I came
up with a solution, but it is quite ugly and not at all perfect
(exception thrown in finally is still "hidden" if an exception
is thrown in try).

There is certainly a trade-off here. The code you've written (assuming
that you meant to set 'errors' to false at the end of the try block)
certainly gives information that's ever so slightly more likely to be
useful, but at the cost of considerable extra complexity in the code
itself.

I just can't imagine an IOException where I would care whether the
exception happened in the body of the try block or in the finally block.
If I could imagine such a thing, I might think that the extra complexity
is worth it.

On the other hand, you're again backing up and moving exception handling
strategies to the wrong place. Let's say, hypothetically, that my code
calls your code first, and if your code fails, then my code chooses a
different strategy to accomplish the same goal. So if your code fails,
that is NOT a user-level error; yet with the code you've provided here,
an exception message might appear in a log (which is a user-level
concept) claiming that there was an error closing a file. That's not
the correct behavior.

No. The earlier exception is much more important, since it
also contains information about the state in which the (please
forgive me) "business logic" was when things went wrong. This
is usually quite important especially if fixing the problem
involves cleaning up things that were left in an invalid state.

Can you be more concrete? The state of the method or object won't
change because of an attempt to call close. The only thing you lose is
the exact line of code on which the original exception occurred. On the
other hand, you do find out the line of code on which the second
exception occurred. Again, I don't see a compelling reason to prefer
the first exception to the second.

--
www.designacourse.com
The Easiest Way To Train Anyone... Anywhere.

Chris Smith - Lead Software Developer/Technical Trainer
MindIQ Corporation

 

[Mark 'Kamikaze' Hughes]

That's not a problem if the whole project is standardised on Chris'
preferred indentation style. I used to use the Sun indentation style when
I started programming in Java, but every place I've worked has
standardised on the other way, the one that Chris prefers - and I have to
say that I now prefer this way too. The Sun style seems to be dominant in
academic institutions, but outside of that it seems a pretty even mix of
the two in my experience.

I've never used Java in an academic environment, but at every large
corporation I've done Java for over the last 9 years, it's universally
been mandated that you should use Sun's style, and most small companies
do, too--the rest simply don't care, and so everyone ends up using it
because it's the only common language.

Again, NOBODY en the entire world cares about your personal artistic
expression through indentation. It's crazy to go irritating your
co-workers with a random alternate style.

 

[Mark 'Kamikaze' Hughes]

I'll try to slow down a bit, then. If you catch the exception before
you are in a position to handle it, then you are left with the following
options:

You're always in a position to handle it. If not, you've made some
major design errors.

Quite the contrary, it is of course very possible to catch exceptions in
the AWT event thread, in multithreaded environments, etc. Sometimes
it's less obvious how to go about it, but I haven't seen a situation
where it's entirely impossible.

Great. Tell me where that exception should be caught, then. Go
ahead. Oh, wait, you can't, because it's not possible to catch AWT
exceptions except at the level where it re-enters application code. You
have to handle it close to the origin.

Frankly, I don't care what you have to say. You've proven yourself to
be an ass, and I don't care to continue the conversation. Bye.

That's rich, coming from you. If you don't like being called on your
juvenile behavior, you probably shouldn't indulge in it.

 

[Antti S. Brax]

Antti S. Brax <[email protected]> wrote:

strategies to the wrong place. Let's say, hypothetically, that my code
calls your code first, and if your code fails, then my code chooses a
different strategy to accomplish the same goal. So if your code fails,
that is NOT a user-level error; yet with the code you've provided here,
an exception message might appear in a log (which is a user-level
concept) claiming that there was an error closing a file. That's not
the correct behavior.

Well, instead of logging the second error message I can just
ignore it. No user level concepts involved anymore.

Can you be more concrete? The state of the method or object won't
change because of an attempt to call close. The only thing you lose is
the exact line of code on which the original exception occurred.

In this example we lose the line of code, since the exception
class hierarchy in the java.io package is so braindead. If the
exceptions contained information about the nature of the error,
then we would lose more information.

It's a trade off, alright. And I am not willing to make strict
generic rules based on this example. In future I will decide
case by case which exception is more important and will never
again automatically catch everything in the finally block.

 

[Daniel Dyer]

Again, NOBODY en the entire world cares about your personal artistic
expression through indentation. It's crazy to go irritating your
co-workers with a random alternate style.

I'm not disagreeing, but if everybody on the team agrees to use the "other
way", there's not a problem, is there? They'll only get irritated if you
start using the Sun convention when everybody else is using the "random
alternate style".

I switched to the opening braces on new-line approach because at the first
company I worked at the entire development team was expressing their
personal artistic preferences identically with this "random alternate
style" (although they didn't call it this, they called it the coding
standards).

Dan.

 

[Chris Smith]

Great. Tell me where that exception should be caught, then. Go
ahead. Oh, wait, you can't, because it's not possible to catch AWT
exceptions except at the level where it re-enters application code. You
have to handle it close to the origin.

Mark probably won't care since he's just here to troll. However, for
the benefit of others, the following code very effectively catches
exceptions occurring in the AWT event thread:

public static void main(final String[] args)
{
new Thread(group, new Runnable() {
public void run()
{
Toolkit.getDefaultToolkit().getSystemEventQueue()
.push(new EventQueue() {
protected void dispatchEvent(AWTEvent event)
{
try
{
super.dispatchEvent(event);
}
catch (Exception e)
{
handleException(e);
}
}
});

startApp(args);
}
}).start();
}

public static void startApp(String[] args)
{
// Normal application code goes here
}

public static void handleException(Exception e)
{
// Generic exception handling goes here.
}
--
www.designacourse.com
The Easiest Way To Train Anyone... Anywhere.

Chris Smith - Lead Software Developer/Technical Trainer
MindIQ Corporation

 

[Mark 'Kamikaze' Hughes]

I'm not disagreeing, but if everybody on the team agrees to use the "other
way", there's not a problem, is there? They'll only get irritated if you
start using the Sun convention when everybody else is using the "random
alternate style".

That's correct. If the team is using some other style, then you
should use that style, too.

However, in practice in enterprise companies, I've never seen another
style in use, and with IDE support for code styles defaulting to Sun's
conventions, it's increasingly unlikely to be anything else.

This started with Chris "correcting" someone else's code style into
his own artistic vision, which is exactly what you should never ever do.

 

[Lee Fesperman]

That's correct. If the team is using some other style, then you
should use that style, too.

Good, a rational response.

However, in practice in enterprise companies, I've never seen another
style in use, and with IDE support for code styles defaulting to Sun's
conventions, it's increasingly unlikely to be anything else.

My experience is just the opposite. I've never seen enterpise companies using the
Sun/K&R style. IDE defaults shouldn't drive code styles.

Personally, I find Sun/K&R very hard to read and to match bracing. IDE's shouldn't drive
coding style. I love "random alternate style". Whitespace improves understanding.

This started with Chris "correcting" someone else's code style into
his own artistic vision, which is exactly what you should never ever do.

Maybe if he was working on a group project, but this is usenet. Actually, we do a lot of
code correcting here. Thank you, Chris, I don't read usenet code in an IDE.

 

[Mark 'Kamikaze' Hughes]

Good, a rational response.

My responses have always been rational; you might not have *liked*
them, but the truth is often painful. However, I'd worry that you'd
take that very improbable *IF* clause above as license to annoy your
co-workers with your artistic expression.

My experience is just the opposite. I've never seen enterpise
companies using the Sun/K&R style. IDE defaults shouldn't drive code
styles.

The IDE defaults are set that way for a reason, it's not just a
coincidence or following some unpopular style. K&R set their style that
way for a good reason.

You can go ahead and survey companies if you like, and you will find
that the Sun style guide is followed almost all the time.

Personally, I find Sun/K&R very hard to read and to match bracing.
IDE's shouldn't drive coding style. I love "random alternate style".
Whitespace improves understanding.

<boggle> Vertical whitespace actively interferes with reading code,
because it steals valuable vertical space. You have at least 80
columns, but at most 30-odd lines if you want them to be readable, and a
function that's more than a screen long becomes exponentially harder to
understand. This isn't exactly news, this has been known for over 30
years.

You get no benefit from the vertical braces. They duplicate
information that you get from the indentation level (possibly
erroneously), and give you the opportunity to cause horrible bugs you'll
never find, because nobody who uses vertical braces uses them all the
time. With the K&R style, you can afford the screen space to always use
braces, and make safer code.

Maybe if he was working on a group project, but this is usenet.
Actually, we do a lot of code correcting here. Thank you, Chris, I
don't read usenet code in an IDE.

It's still unbelievably rude, and sets a bad example, as seen here,
with people actually trying to justify this bad behavior. Terrifying.

 

[Daniel Dyer]

<boggle> Vertical whitespace actively interferes with reading code,
because it steals valuable vertical space. You have at least 80
columns, but at most 30-odd lines if you want them to be readable, and a
function that's more than a screen long becomes exponentially harder to
understand. This isn't exactly news, this has been known for over 30
years.

There may be a grain of truth in this but I'd be very surprised if the
effect were "exponential". Many people will tell you that if your method
doesn't fit on a screen it should be refactored anyway, so regardless of
your whitespace preferences, you shouldn't be doing much scrolling within
a single method declaration.

I'm pretty sure that vertical whitespace has not hindered my ability to
understand my code. I find it helpful sometimes to group together related
statements within a block by putting blank lines between statements. I
also like to put at least two blank lines rather than one between method
declarations. But I don't see it as stealing. It's a victimless crime,
my company can afford the disk space taken up by the new-line characters.

You get no benefit from the vertical braces.

I would argue that in some cases it's clearer to have the opening brace on
the left (so you don't have to move your eyes to the right to see if you
are in a new block or not). But I'm not arguing that the vertical style
is the "one true brace style", just that either approach is just as valid
as the other and the vertical one happens to be my preference.

They duplicate
information that you get from the indentation level (possibly
erroneously), and give you the opportunity to cause horrible bugs you'll
never find

Can you provide an example of one of these horrible bugs that I'll never
find?

because nobody who uses vertical braces uses them all the time.

I use the vertical brace style all the time (I've just run checkstyle on
my current project to confirm this). Have you any reason to believe that
people using this style are are any less consistent in their brace
placement than those using the Sun conventions?

With the K&R style, you can afford the screen space to always use
braces, and make safer code.

I agree with the always using braces, and I can still "afford" this using
an extra new-line for every block.

Dan.

 

[Patrick May]

The IDE defaults are set that way for a reason, it's not just a
coincidence or following some unpopular style. K&R set their style
that way for a good reason.

Yes, because it takes up fewer lines in a printed book.

You can go ahead and survey companies if you like, and you will
find that the Sun style guide is followed almost all the time.

This is definitely not my experience. I've been in the industry
for over 20 years and coding in Java since it first became available.
I see a roughly 60-40 split in various companies and projects, with
some variant of "open brace on a newline" in the majority.

Sincerely,

Patrick

 

[Dale King]

This started with Chris "correcting" someone else's code style into
his own artistic vision, which is exactly what you should never ever do.

So if the OP's original code looked like this:

try { fos = new FileOutputStream
(FILENAME); ... // write to file
} finally {
if (fos
!= null)
fos.close();
}

Are you really saying that it would be wrong for him to correct the code
to make it readable?

While we can argue whether or not the opening brace should be hidden at
the end of the line (does that tell you which I choose), the real
problem with the OP's code that he corrected was the line: "} finally {"
which I find atrocious.

And I agree with Lee. My experience is that very few use Sun's hidden
brace style.

My feelings on conventions is that they have to be justified by
reasoning why that convention is best. Just that Sun chooses something
does not make it binding on others. Sun provides no justification for
their convention and there is plenty of justification for balancing the
braces.