need argument for try catch blocks

[Christopher]

I've got my self an old c style programmer reviewing my code at work.

I've already discovered a plethora of bugs in that exceptions are
getting thrown from native code, the windows API, and our own code and
never handled.

However, in my code reviews, I've got items about "unecessary try
catch blocks." I really don't know why it is even worthy of putting in
a code review. Maybe he is thinking efficiency or readability?

My point of view is that when your dealing with a huge project with
poor documentation, it is better to always surround anything that
throws with a try catch block, even if it seems unecessary. I can't
count the number of hours I've spend debugging code that was bugged
because of no error checking. The code changes all the time and a
catch that might never get hit, could get hit after many edits. If
nothing else it seems like a good way to assist the poor guy
debugging. It says to me, "Hey an error could occur here!"

My problem is in convincing my peers. I cannot formalize my argument.
Is there an source with clout that will back my argument up? Or
perhaps someone can put into better words what I am describing? Or am
I just wrong?

 

[Balog Pal]

I've got my self an old c style programmer reviewing my code at work.

Being C programmer helps little using C++, it is more like a hindrance. To
be good in C++ you shall unlearn very much.

I've already discovered a plethora of bugs in that exceptions are
getting thrown from native code, the windows API, and our own code and
never handled.

Unhandled exception is not a coding mistake but part of fundamental design.
The project shall have an exception handling policy that makes it clear
where the boundaries are and where to catch. Making stuff obvoius. And
wothout no wonder you have chaos.

However, in my code reviews, I've got items about "unecessary try
catch blocks."

Naturally. Catch supposed to be very rare. And restricted to library border
and high level functions.

I really don't know why it is even worthy of putting in
a code review. Maybe he is thinking efficiency or readability?

Likely just know how code is supposed to be. The norm is to be
exception-neutral. Possibly with some odd throws. If there is a catch, the
content shall make it obvious that it is necessary. If there is doubt it
shall not be there.

My point of view is that when your dealing with a huge project with
poor documentation, it is better to always surround anything that
throws with a try catch block, even if it seems unecessary.

Huh? Hodgepodge will not make anything better. Adding lines without a clear
and sensible reason just increases the mess. For exceptions it is even
more so, as they propagate finely. So the only code that catch is the one
that knows too well what it expects and what to do about it. And why,
obviously.

I can't
count the number of hours I've spend debugging code that was bugged
because of no error checking.

Exceptions make it quite impossible to "not check" in the C sense, so your
case can not apply. The most that can happen the program is hit with
exception no one thought to handle, where you get terminate() as the best
default.

But to have anything better you shall be able to show the point between
emitter and main() where it was supposed to do anything else to call it a
coding bug. Just inserting random try blocks at random places is not that.

The code changes all the time and a
catch that might never get hit, could get hit after many edits. If
nothing else it seems like a good way to assist the poor guy
debugging. It says to me, "Hey an error could occur here!"

Care to explain how it says that at the random catch site? And how you can
state it is "error", when upstream someone could have the proper catch?

My problem is in convincing my peers.

No wonder.

I cannot formalize my argument.

Don't formalize then. Just try to make your case, say here. In a real sense.
Show some "broken" code and your "fixed" version. Explain the difference,
showing the situations when it is better. Then be ready to defend your
position.

Is there an source with clout that will back my argument up?

We will see as soon as "argument" is actually presented.

Or
perhaps someone can put into better words what I am describing? Or am
I just wrong?

Most likely the latter, but do make your case, so can learn either this or
that way.

 

[Christopher]

Being C programmer helps little using C++, it is more like a hindrance. To
be good in C++ you shall unlearn very much.


Unhandled exception is not a coding mistake but part of fundamental design.
The project shall have an exception handling policy that makes it clear
where the boundaries are and where to catch. Making stuff obvoius. And
wothout no wonder you have chaos.


Naturally. Catch supposed to be very rare. And restricted to library border
and high level functions.


Likely just know how code is supposed to be. The norm is to be
exception-neutral. Possibly with some odd throws. If there is a catch, the
content shall make it obvious that it is necessary.  If there is doubt it
shall not be there.


Huh? Hodgepodge will not make anything better. Adding lines without a clear
and sensible reason just increases the mess.   For exceptions it is even
more so, as they propagate finely.  So the only code that catch is the one
that knows too well what it expects and what to do about it. And why,
obviously.


Exceptions make it quite impossible to "not check" in the C sense, so your
case can not apply.  The most that can happen the program is hit with
exception no one thought to handle, where you get terminate() as the best
default.

But to have anything better you shall be able to show the point between
emitter and main() where it was supposed to do anything else to call it a
coding bug.  Just inserting random try blocks at random places is not that.


Care to explain how it says that at the random catch site? And how you can
state it is "error", when upstream someone could have the proper catch?


No wonder.


Don't formalize then. Just try to make your case, say here. In a real sense.
Show some "broken" code and your "fixed" version.  Explain the difference,
showing the situations when it is better. Then be ready to defend your
position.


We will see as soon as "argument" is actually presented.


Most likely the latter, but do make your case, so can learn either this or
that way.

Well, I don't really view it as "random" catch site. I put a catch on
anything that throws.

A few examples:

1) A dll that contains multithreaded code that is part of a window's
service contains a method that uses boost::lexical_cast on a string
that came in as a parameter.

I'd surround the lexical cast with a try-catch. If boost threw, I
create an error message complaining that the parameter contained
invalid text, log it, and throw a (company made) invalid argument
exception. Supposedly, the code higher up will catch this and
determine if the action it was trying to take requires program
termination, more logging, or what to do.

2) Same scenario except I am casting a static constant integer to a
string.
My argument is that later on that static constant integer may no
longer be. The design is changing, things get edited all the time, and
I don't see the harm in providing some clarity for the next guy that
"hey boost can throw here!", rather than leaving them to look up every
single function call in various referenced and checking if they throw
or not.

3) A constructor of an object makes a connection to a database. If the
database connection fails, the object cannot be constructed. Thus, I
throw an exception with a "DB connection failure" error. I then catch
it in the method that tryed to construct the object. He would argue to
let the exception travel on up. I'd argue that it would be better to
catch it there to let other programmers know that construction can
fail
due to a database connection error.


A huge problem in this project is that somebody at some time had the
mentality of "if any error occurs anywhere, throw an exception and
don't worry about releasing resources, catching and handling, or
service termination." They put some 3rd party library at the top that
catches any unhandled exceptions and logs them. But they didn't seem
to care about the undefined state it left the system in!

I am trying to systematically fix that as I go throw each source file.
To do it at once seems an impossible task. I think a beginning is to
start writing try/catch blocks on a case for case basis and determine
how to handle the error conditions one at a time. I'd rather
arbitralilly catch, than arbitrallily let an exception go up the stack
and take it for granted that resources were released and someone else
is catching every flavor of exception and handling it!

However, it seems you might have a different view in that an exception
should be allowed to travel up the stack in some cases where it needs
to be well-defined in the design where they should be handled. We
don't have any such design in place. So, what does a lowly minion with
barely any input on design, do in that case?

 

[Christopher]

Well, I don't really view it as "random" catch site. I put a catch on
anything that throws.

A few examples:

1) A dll that contains multithreaded code that is part of a window's
service contains a method that uses boost::lexical_cast on a string
that came in as a parameter.

I'd surround the lexical cast with a try-catch. If boost threw, I
create an error message complaining that the parameter contained
invalid text, log it, and throw a (company made) invalid argument
exception. Supposedly, the code higher up will catch this and
determine if the action it was trying to take requires program
termination, more logging, or what to do.

2) Same scenario except I am casting a static constant integer to a
string.
My argument is that later on that static constant integer may no
longer be. The design is changing, things get edited all the time, and
I don't see the harm in providing some clarity for the next guy that
"hey boost can throw here!", rather than leaving them to look up every
single function call in various referenced and checking if they throw
or not.

3) A constructor of an object makes a connection to a database. If the
database connection fails, the object cannot be constructed. Thus, I
throw an exception with a "DB connection failure" error. I then catch
it in the method that tryed to construct the object. He would argue to
let the exception travel on up. I'd argue that it would be better to
catch it there to let other programmers know that construction can
fail
due to a database connection error.

A huge problem in this project is that somebody at some time had the
mentality of "if any error occurs anywhere, throw an exception and
don't worry about releasing resources, catching and handling, or
service termination." They put some 3rd party library at the top that
catches any unhandled exceptions and logs them. But they didn't seem
to care about the undefined state it left the system in!

I am trying to systematically fix that as I go throw each source file.
To do it at once seems an impossible task. I think a beginning is to
start writing try/catch blocks on a case for case basis and determine
how to handle the error conditions one at a time. I'd rather
arbitralilly catch, than arbitrallily let an exception go up the stack
and take it for granted that resources were released and someone else
is catching every flavor of exception and handling it!

However, it seems you might have a different view in that an exception
should be allowed to travel up the stack in some cases where it needs
to be well-defined in the design where they should be handled. We
don't have any such design in place. So, what does a lowly minion with
barely any input on design, do in that case?- Hide quoted text -

- Show quoted text -

Wow do I have spelling errors. I wish you could edit newsgroup
posts
Sorry, I hope it still makes sense. I can provide clarity if it is
needed.

 

[Balog Pal]

Well, I don't really view it as "random" catch site. I put a catch on
anything that throws.

Bad Thing (TM).

Error codes are for communication with the immediate caller. Exceptions are
for communication long distance. Several callers upward typically. It is
like checking passports on every corner instead of doing that only at the
country borders.

1) A dll that contains multithreaded code that is part of a window's
service contains a method that uses boost::lexical_cast on a string
that came in as a parameter.

I'd surround the lexical cast with a try-catch. If boost threw, I
create an error message complaining that the parameter contained
invalid text, log it, and throw a (company made) invalid argument
exception.

Converting exceptions to others is fair in itself. It may be good too, if
you have that single use of the boost facility. However if your DLL uses
boost profoundly, you better use try blocks on your dllexport functions. And
leave the whole implementation alone.

2) Same scenario except I am casting a static constant integer to a string.

My argument is that later on that static constant integer may no
longer be. The design is changing, things get edited all the time, and
I don't see the harm in providing some clarity for the next guy that
"hey boost can throw here!", rather than leaving them to look up every
single function call in various referenced and checking if they throw
or not.

The more the implementation changes the less you want to maintain the
avalanche. Exception are, uh, exceptional. (provided a correct design).
Normally when you're hit by one, you can't do much but cancel a full
high-level operation (or the whole process). Consequently it is enough to
handle them at those few locations. Collectively. And in a simple, stable
way.

As i said, converting from one exception to another is fair, but consider if
there is a benefit really.
Logging is a generic excuse, but IME is way less helpful than claimed. The
natural point is again at the top level, you state the imput params,
possibly the environment and the content of the caught exception. If you
skip that, and only log at a lower point, you'll know what happened but not
why. While having both is mostly redundant.

3) A constructor of an object makes a connection to a database. If the
database connection fails, the object cannot be constructed. Thus, I
throw an exception with a "DB connection failure" error.

Sure. ctors are common exception sources.

I then catch
it in the method that tryed to construct the object.

Why on Earth?

He would argue to let the exception travel on up.

Definitely.

Is DB opening expected? So let the function expect it, and just do its job.
The module dealing with DB can emit arbitrary amount of exceptions,
practically anywhere. If nothing else, SQL timeout is beyond your control,
and can happen in any (otherwise correct) operation. And you have no
realistic way to recover at low level. While on the high level youre not
interested at all which of the 1298 operation failed.

I'd argue that it would be better to
catch it there to let other programmers know that construction can
fail due to a database connection error.

Heh? If you want to teach programmers, do it in the proper institution.
Catching exceptions are not done for teaching puurposes. the only purpose to
catch the exception is to do some relevant action.

In your example action WHAT do you do in the catch block?

A huge problem in this project is that somebody at some time had the
mentality of "if any error occurs anywhere, throw an exception and
don't worry about releasing resources, catching and handling, or
service termination."

Certainly all your functions must bring at least the "basic guarantee" from
the Abrahams classifications.
If that holds, the statement is right: you shall not worry about anything.
Resources are handled by RAII. Exceptions are caught upstream where makes
sense. Your job is to stay out of the way, and do the actual work.

They put some 3rd party library at the top that
catches any unhandled exceptions and logs them.
But they didn't seem
to care about the undefined state it left the system in!

Maybe we're getting somewhere. If the program is in undefned state that is a
problem indeed. But instead of infesting the program with catch clause, you
shall concentrate on that alone: reviwe and reorganize the functions that
are not exception safe.

Hint: it is NOT done using try blocks. But by using RAII, the swap idiom
and stuff like that. mainly by avoiding to mess up the state in the first
place, and break invariants.

I am trying to systematically fix that as I go throw each source file.

I'm still not convinced your fix counts as one. I saw such attempts and
most only pushed the problem and made it subtler, instead of eliminating the
roots. did you ate least some fundamental literatire on exception safety,
and how to reach it in C++? I recently suggested someone to check out
gotw.ca.

To do it at once seems an impossible task. I think a beginning is to
start writing try/catch blocks on a case for case basis and determine
how to handle the error conditions one at a time.

No, the beginning is to learn the fundamental C++ idioms. Aim, set, fire.
Not the other way around.

I'd rather
arbitralilly catch, than arbitrallily let an exception go up the stack
and take it for granted that resources were released and someone else
is catching every flavor of exception and handling it!

If you have non-controlled resources around your fight is futile. And if you
want to fix the situation start RAII-zing the stuff as first step.

However, it seems you might have a different view in that an exception
should be allowed to travel up the stack in some cases where it needs
to be well-defined in the design where they should be handled.

In a healthily organized code you can actually do as your quote above:
return or throw at any (almost) point of the code, and have sensible
outcome. And if the code is not like that, you're doomed anyway.

We don't have any such design in place.

So another starting point is to have it. Actually THAT is the thing that
is supposed to teach and guide the programmers.

So, what does a lowly minion with
barely any input on design, do in that case?

Do you really want my suggestion? In such a situation I would either fix it
in the real sense, or quit for good. Actually did a plenty of both.

 

[Juha Nieminen]

However, in my code reviews, I've got items about "unecessary try
catch blocks." I really don't know why it is even worthy of putting in
a code review. Maybe he is thinking efficiency or readability?

In general try/catch blocks do not make the code any slower, so an
efficiency argument is unjustified.

My point of view is that when your dealing with a huge project with
poor documentation, it is better to always surround anything that
throws with a try catch block, even if it seems unecessary. I can't
count the number of hours I've spend debugging code that was bugged
because of no error checking. The code changes all the time and a
catch that might never get hit, could get hit after many edits. If
nothing else it seems like a good way to assist the poor guy
debugging. It says to me, "Hey an error could occur here!"

If an exception is thrown, it will be caught somewhere (or thrown out of
main(), which is less than ideal). Any good debugger will tell you where
the exception was thrown from (and the stack trace).

 

[Stefan Ram]

I've already discovered a plethora of bugs in that exceptions are
getting thrown from native code, the windows API, and our own code and
never handled.

Not »handling« an exception does not have to be a bug.

However, in my code reviews, I've got items about "unecessary try
catch blocks." I really don't know why it is even worthy of putting in
a code review. Maybe he is thinking efficiency or readability?

Who? One uses the personal pronoun »he« after a person has
been introduced.

So, if you already know »him«, why can't you ask him?

My point of view is that when your dealing with a huge project with
poor documentation, it is better to always surround anything that
throws with a try catch block, even if it seems unecessary.

This is exacly what exceptions were invented to prevent.

I can't count the number of hours I've spend debugging code
that was bugged because of no error checking.

Exceptions already alter the control flow, so that an
explicit »error check« often is not necessary.

Is this application code or library code?

What is your idea of what to do in the catch block?

Or am I just wrong?

I suggest that you read the chapters on exceptions in some
text books.

 

[Edek]

I've got my self an old c style programmer reviewing my code at work.

I've already discovered a plethora of bugs in that exceptions are
getting thrown from native code, the windows API, and our own code and
never handled.

However, in my code reviews, I've got items about "unecessary try
catch blocks."

What do those blocks do when they catch something?

If they do anything that is necessary or of value, they should be there.
Naturally, the project formal/informal policy is one thing to follow,
but in general such 'extra' try-catches are done for example:

a) to clean up resources (like DB, if destructors are not enough), then
they are necessary
b) to rethrow as another exception - utility really, see d)
c) to log, if there are no stack traces and for some reason a debugger
cannot be used, or just it is quicker to have a log without plugging in
a debugger
d) to work around caller code, which does not really handle exceptions,
or handles just a few types - then catching is better than terminate()

I really don't know why it is even worthy of putting in
a code review. Maybe he is thinking efficiency or readability?

Efficiency? No. Readability? It depends, mainly on what the reader is
used to. Different people, different habits.

My point of view is that when your dealing with a huge project with
poor documentation, it is better to always surround anything that
throws with a try catch block, even if it seems unecessary. I can't
count the number of hours I've spend debugging code that was bugged
because of no error checking. The code changes all the time and a
catch that might never get hit, could get hit after many edits. If
nothing else it seems like a good way to assist the poor guy
debugging. It says to me, "Hey an error could occur here!"

I find that very true. Even if no exception can be throw now, some time
from now it may be thrown because of callee changes, so to be on a safe
side, it is good to try/catch. Good maintained code is usually handling
errors nicely 'by design', no matter when the faults are introduced.

My problem is in convincing my peers. I cannot formalize my argument.
Is there an source with clout that will back my argument up? Or
perhaps someone can put into better words what I am describing? Or am
I just wrong?

HTH
Edek

 

[Balog Pal]

If they do anything that is necessary or of value, they should be there.
Naturally, the project formal/informal policy is one thing to follow, but
in general such 'extra' try-catches are done for example:

a) to clean up resources (like DB, if destructors are not enough), then
they are necessary

Hell no. Resources must be handler by RAII, and never left to hang around
"raw". And mandatory release code must not be multiplicated at every
possible exit.

c) to log, if there are no stack traces and for some reason a debugger
cannot be used, or just it is quicker to have a log without plugging in a
debugger

Logging is a suspicious reason, and if really needed also can be done better
in a RAII-like form. Though special cases may apply.

d) to work around caller code, which does not really handle exceptions, or
handles just a few types - then catching is better than terminate()

I'd rather call it "when exceptions must be converted to error return codes
for the caller".

 

[Edek]

Hell no. Resources must be handler by RAII, and never left to hang
around "raw". And mandatory release code must not be multiplicated at
every possible exit.

Is it possible to do anything which may throw during stack unwind? I
always thought that was UB.

Logging is a suspicious reason, and if really needed also can be done
better in a RAII-like form. Though special cases may apply.

Like having no exception stack trace? Hell, I like getting at the top of
the thread an information along the lines of 'Integer conversion failed'
and nothing more.

I'd rather call it "when exceptions must be converted to error return
codes for the caller".

Sure.

 

[Keith H Duggar]

Bad Thing (TM).

Error codes are for communication with the immediate caller. Exceptions are
for communication long distance. Several callers upward typically.  It is
like checking passports on every corner instead of doing that only at the
country borders.

That's a good analogy. Life Would Suck (TM) if passports were
checked on every corner. And code that checks every statement
/individually/ for exceptions Sucks For Sure.

KHD

 

[Balog Pal]

Is it possible to do anything which may throw during stack unwind? I
always thought that was UB.

Depends. If you use the standard library, it states that you'll have UB if
your type T emits an exception from a destructor.

In general that is not UB, but a double exception will end up in
terminate(). And a throwing destructor can leave behind memory leaks,
incosistencies and much other stuff you do not want. for that reason it is
a strong guideline that destructors must be nothrow. Those ignoring it are
likely in peril regardless other stuff.

Like having no exception stack trace? Hell, I like getting at the top of
the thread an information along the lines of 'Integer conversion failed'
and nothing more.

If you write correct software, you will not have that. If it is caused by
not software but input external input, then you can dump the input at that
place, and figure out why it was not fit for purpose.

As I told before, chasing more log and traci is a usual activity that
correlates tith WTF-ish codebase. The rrot fix of that is not adding more
trace support but start work on reviews and fixes.

Whily really hairy environments support externally controllable trace. Like
the linux kernel -- you can echo > to turn on tracepoints, debug messages on
a single module, or even on a single line of source if interested. And it is
NOT there for the regular case. Neither needs excess code for tracing (only
for special messages).

And adding noise to the code provably hinders making it right. While making
it simpler and easier to read leads to less problems to look after. Or
putting the same effort into unit tests will more likely find the cause of
your integer conversion.

 

[Edek]

Depends. If you use the standard library, it states that you'll have UB
if your type T emits an exception from a destructor.

In general that is not UB, but a double exception will end up in
terminate(). And a throwing destructor can leave behind memory leaks,
incosistencies and much other stuff you do not want. for that reason it
is a strong guideline that destructors must be nothrow. Those ignoring
it are likely in peril regardless other stuff.

Yes, that I know. Actually, what I mean is this

struct Deleter {
... holds a DB connection

~Deleter () {
try {
.. whatever that may throw
} catch (...) {} // does not emit. But is it legal?
}

int some {
scoped_ptr<Deleter> del(new Deleter(openDbConnection()) );

// ... here something throws
// destructor throws and catches
// is this UB?
}

Is this UB or not? I've always thought it is.

If you write correct software, you will not have that. If it is caused
by not software but input external input, then you can dump the input at
that place, and figure out why it was not fit for purpose.

Ok, integer conversion is just an example, could be anything.

As I told before, chasing more log and traci is a usual activity that
correlates tith WTF-ish codebase. The rrot fix of that is not adding
more trace support but start work on reviews and fixes.

Whily really hairy environments support externally controllable trace.
Like the linux kernel -- you can echo > to turn on tracepoints, debug
messages on a single module, or even on a single line of source if
interested. And it is NOT there for the regular case. Neither needs
excess code for tracing (only for special messages).

Yes, like entering an important function pushes something on TLS log
context, or whatever via RAII. I know. Somewhere in this thread there
was something said about hard debugging.

And adding noise to the code provably hinders making it right. While
making it simpler and easier to read leads to less problems to look
after. Or putting the same effort into unit tests will more likely find
the cause of your integer conversion.

Sure, there is a fake tradeoff between quality and effort. Quality here
would mean that there is no need (or place) for weird stuff, and
debugging would be easy. That's when a project has a reasonable policy,
in this case of error handling, even in the midst of various libraries.
The tradeoff works in the short run, in the long run is a pain, I'm not
sure about the overrated TCD (Total Cost of Development).

But this is more about the project in large. I do prefer raii, but it
needs consistency.

Edek

 

[Balog Pal]

Yes, that I know. Actually, what I mean is this

struct Deleter {
... holds a DB connection

~Deleter () {
try {
.. whatever that may throw
} catch (...) {} // does not emit. But is it legal? }

Yes, that is perfectly legal. The standard tays that if a dtor called in the
stach unwinding *exits* through an exception, then terminate() is called. So
you are supposed to catch and swallow them. (see section 15.2 or .3)

Some libraries use uncaught_exception() in dtors and only swallow if it is
true. IIRC that is not a surefire way and the return value is not in exact
match with that intent, but I forgot the details. But as alrady mentoned,
emitting dtor has problems in other contexts too, so the above code makes
general sense, if anything not nothrow() is actually called.

The only realistic bad case I recall if for CFile used as output and not .
Close()-d in MFC 4.2 (might got fixed for more current versions).

int some {
scoped_ptr<Deleter> del(new Deleter(openDbConnection()) );

// ... here something throws
// destructor throws and catches
// is this UB?
}

Is this UB or not? I've always thought it is.

With the try-and swallow it is okay.

for the standard library the applicable part is at 17.4.3.6 bullet 4
In particular, the effects are undefined in the following cases: ...

- if any replacement function or handler function or destructor operation
throws an exception, unless specifically allowed in the applicable Required
behavior paragraph.

This only applies to exceptions actually escaping.

 

[Christopher]

Hell no. Resources must be handler by RAII, and never left to hang around
"raw". And mandatory release code must not be multiplicated at every
possible exit.


Logging is a suspicious reason, and if really needed also can be done better
in a RAII-like form. Though special cases may apply.


I'd rather call it "when exceptions must be converted to error return codes
for the caller".

I'd love to be counting on destructors. So far though, I'd say 60% of
the code is written in C-style, with global pointers being to things
allocated in various class methods, released in others, global
instances being used left and right, macros, etc. etc. Things are just
not well contained. There are classes where the functionality is
partly handled by global functions and partly handled by class
methods. But, I am ranting.

My point is though, out here, at least in my city and for several jobs
now, there always seems to be that c-style programmer on the team that
does this kind of thing ruining or at least making it difficult to
follow most of the OO principles. They are usually well respected with
seniority too. Things get into such a mess that I don't know where to
start fixing it. I wish I could just mind control someone and convince
them to follow S.O.L.I.D principles, RAII, and several other things.

Anyway, the advice in the posts is good. You guys have changed my mind
on exception handling. Maybe I will try to convice my team that we
should take the time to come up with a design decision on when and how
to handle exceptions, their types, identify these boundries, etc.

 

[Stefan Ram]

Depends. If you use the standard library, it states that you'll have UB if
your type T emits an exception from a destructor.

One can always call anything from within a destructor. One
then just has to wrap everything with »try« and »catch«, so
that exceptions /do not leave/ the destructor.

 

[Balog Pal]

I'd love to be counting on destructors. So far though, I'd say 60% of
the code is written in C-style, with global pointers being to things
allocated in various class methods, released in others, global
instances being used left and right, macros, etc. etc. Things are just
not well contained. There are classes where the functionality is
partly handled by global functions and partly handled by class
methods. But, I am ranting.

Indeed. What you say is all the MORE reason to start reshaping code so that
all resources have an assigned handler. You have NO chance to get rid of
leaks otherwise. Only to shift them around and make subtler. Dragging in a
swamp of code in the meantime that could be not there at all.

My point is though, out here, at least in my city and for several jobs
now, there always seems to be that c-style programmer on the team that
does this kind of thing ruining or at least making it difficult to
follow most of the OO principles.

Yes, it is common. As we know like 90% of software products are crapware,
and still like 50% fails to deliver at all. Life is hard. It will be even
harder if you join the bandwagon and ADD to problems instead of fight to
reduce them. If you fail in tha latter, at least can look in the mirror
and say that you did try to do the right thing. And eventually may hit a
place where you succeed. As opposed to keep working in the latrine
endlessly.

They are usually well respected with seniority too.

Yep, as incompetene start at managers, and "respect" too often means "suck
up" instead of any original meanings. While "seniority" measured in years
passed instead of working stuff created.

Sorry, can't tell you anything bright. OTOH you're your own life's master,
and *can* shape what happens with you.

Things get into such a mess that I don't know where to
start fixing it. I wish I could just mind control someone and convince
them to follow S.O.L.I.D principles, RAII, and several other things.

First learn who can be convinvced. Many people are locked in their ideas
and will not budge. Even by showing working stuff. (loop up NIH). Do not
waste time on them, but go around. While those who can be convinced
eventually will.

Anyway, the advice in the posts is good. You guys have changed my mind
on exception handling. Maybe I will try to convice my team that we
should take the time to come up with a design decision on when and how
to handle exceptions, their types, identify these boundries, etc.

You have much better chance if you write a policy yourself in a shape that
works, then push it through, possibly with some tweaking. Waiting for
non-interested parties to form a committee and a committee to create
anything useful is just recipe for stalling.

 

[Edek]

I'd love to be counting on destructors. So far though, I'd say 60% of
the code is written in C-style, with global pointers being to things
allocated in various class methods, released in others, global
instances being used left and right, macros, etc. etc. Things are just
not well contained. There are classes where the functionality is
partly handled by global functions and partly handled by class
methods. But, I am ranting.

My point is though, out here, at least in my city and for several jobs
now, there always seems to be that c-style programmer on the team that
does this kind of thing ruining or at least making it difficult to
follow most of the OO principles. They are usually well respected with
seniority too. Things get into such a mess that I don't know where to
start fixing it. I wish I could just mind control someone and convince
them to follow S.O.L.I.D principles, RAII, and several other things.

Anyway, the advice in the posts is good. You guys have changed my mind
on exception handling. Maybe I will try to convice my team that we
should take the time to come up with a design decision on when and how
to handle exceptions, their types, identify these boundries, etc.

If 60% of code is C (or C-style), I wouldn't be orthodox about RAII.
Either the developers do have ways to handle debugging and so on, maybe
just slightly home-made, or maybe some ways can be gradually introduced.

Edek

 

[Fred Zwarts]

One can always call anything from within a destructor. One
then just has to wrap everything with »try« and »catch«, so
that exceptions /do not leave/ the destructor.

This works, of course, only if the destructor knows how to handle the
exception.
But what should the destructor do with unknown exceptions?
Ignoring them will cause the program to continue in a undefined state.
Is that better then UB?

So, just wrapping a destructor code in a

try {
...
catch (...) {}

is not a real solution.

For example, if memory is properly managed, a delete should not throw an
exception.
If it does, it may be an indication of memory corruption.
Should the destructor ignore it and let the program continue,
or is it better to let the exception leave the destructor, so that the
program will terminate,
or should it immediately call terminate and not give outer functions a
chance for properly freeing resources?

I am not so sure whether a destructor should never throw.
My code handles only the exceptions I know to handle.
It might be dangerous to catch unknown exceptions and ignore them.

 

[Balog Pal]

This works, of course, only if the destructor knows how to handle the
exception.
But what should the destructor do with unknown exceptions?
Ignoring them will cause the program to continue in a undefined state.

What undefined state? Normally throw happens related to particular object.
With basic guarantee the caller will no the object in question has not
reached the known state. So mey do some action. For dtors, the object will
be gone so that does not apply. Do not confuse unknown state with undefined
behavior.

Is that better then UB?
Definietely.

So, just wrapping a destructor code in a try { ...catch (...) {}is not a
real solution.

Still better than throwing for the general case. As if that happens, the
dtor chain is stopped right there. Is a member object's dtor throws the
other members in the same complete objects will not be called. If dtor
halfway in the array throws, dtors for the rest of the array are not called.
So throwing you disrupt the symmetry and will cause leaks unless very lucky.

For example, if memory is properly managed, a delete should not throw an
exception.
If it does, it may be an indication of memory corruption.

If memory is detectably corrupt then you shall not throw but halt in tracks.
Throw is NOT for handling program bugs but for recoverable runtime
conditions.

Should the destructor ignore it and let the program continue,
or is it better to let the exception leave the destructor, so that the
program will terminate,
or should it immediately call terminate and not give outer functions a
chance for properly freeing resources?

Is it so hard to design dtors to be nothrow? Why should it do anything that
requires check and throw?

The only dtor I recall that could throw was closing file, flushing buffer on
the way that could fail. And leaked to dtor with calling the general Close()
if the file was opened. But it is a sucking design ayway. When you do such
stuff you shall call Close() explicitely and carry on if it survived. If
nuking the object before that; for an implicit call it can just do the core
work, release the resource and swallow. Those interested in completion did
have a chance to force it earlier.

I am not so sure whether a destructor should never throw.

google for expert articles, mentors wrote a plenty of stuff about that, and
damn convincing too.

My code handles only the exceptions I know to handle.

In this case swallowing is the proper handling by default.

It might be dangerous to catch unknown exceptions and ignore them.

So the design shall not rely on dtors throwing in the first place, then said
danger is gone for good. Call every "commit" explicitly, while rollback and
get-rid type operations shall just do the best effort.