A question of programming style - revisited

[Mark]

Apologies for the lengthy posting....
In a previous posting:
http://groups.google.com/groups?hl=...%240%2422605%24cc9e4d1f%40news.dial.pipex.com

I asked a question about how to ensure that some "tidying up" code would be
run at the end of a Method independently of where the method exits. The
consensus was to use try-finally blocks and using blocks: In the former
case, I could put all my "tidying up" code in the finally block and
everything else in the try block. In the case of simply disposing, I could
initialise in a "using" block.

Whilst this mostly met the requirements, I ran into two issues:

1. Putting large amounts of code in the try block is OK and any return
statements therein do indeed cause the code in the finally block to run.
However, the presence of the try block causes any code that would normally
generate an unhandled exception (invariably some buggy code that I've
written!) to keep quiet. In general, I like to handle only the exceptions
that I choose. The rationale being that any other kind of exception I'd
rather know about and fix the bug, which it invariably is. So, wrapping up
great chunks of code into a try block seems to be out of the question.

2. Of course, the using block only disposes of objects that are initialised
in the using statement. However, an example of something I want to happen
just before the Method exists is writing to a TraceListener something like
"Method is now exiting...".

As I see it, there are three solutions:
1. Use deeply nested (i.e. large numbers of) if and switch statements and
simply put the "tidy up" and trace code at the end as one normally would. I
don't like this because the I end up with huge amounst of nested code.

2. Add the tidy up and trace code before each return statement and also at
the end. This keeps the nesting down, but does cause a lot of repetion
within the method.

3. Use the evil(ish) goto! That is, simply add a goto TidyUpStuff and put
all the tidying up and trace code after the TidyUpStuff label which would be
at the end of the method. I'm not an anti-goto Zealot, but I haven't used
goto since the early days of basic and would rather not revisit it if I
don't have to.

Any thoughts?

Cheers
Mark

 

[Cowboy \(Gregory A. Beamer\)]

Apologies for the lengthy posting....
In a previous posting:
http://groups.google.com/groups?hl=...%240%2422605%24cc9e4d1f%40news.dial.pipex.com

I asked a question about how to ensure that some "tidying up" code would be
run at the end of a Method independently of where the method exits. The
consensus was to use try-finally blocks and using blocks: In the former
case, I could put all my "tidying up" code in the finally block and
everything else in the try block. In the case of simply disposing, I could
initialise in a "using" block.

Whilst this mostly met the requirements, I ran into two issues:

1. Putting large amounts of code in the try block is OK and any return
statements therein do indeed cause the code in the finally block to run.
However, the presence of the try block causes any code that would normally
generate an unhandled exception (invariably some buggy code that I've
written!) to keep quiet. In general, I like to handle only the exceptions
that I choose. The rationale being that any other kind of exception I'd
rather know about and fix the bug, which it invariably is. So, wrapping up
great chunks of code into a try block seems to be out of the question.

Not a wise idea. It is very unlikely that every line would possibly fail,
or, if they did, you would be able to catch all of the problems. It is wise
to only wrap code you feel is likely to blow up in a try ... catch ...
finally.

A better option is to unit test with a variety of values. Try negatives on
integers, as well as zeros (very important, esp. in divide by zero
conditions). Then using the debugging tools to watch values. In general,
procedures should be small and specific, for reusability. This also makes
them easier to test and debug.

2. Of course, the using block only disposes of objects that are initialised
in the using statement. However, an example of something I want to happen
just before the Method exists is writing to a TraceListener something like
"Method is now exiting...".

There are times you have to explicitly call Dispose(), so do not be afraid
to leap on it.

As I see it, there are three solutions:
1. Use deeply nested (i.e. large numbers of) if and switch statements and
simply put the "tidy up" and trace code at the end as one normally would. I
don't like this because the I end up with huge amounst of nested code.

I would not opt for this in most situations. It ends up with unreadable
code. The exception is if you have input and output checks in a procedure.
The ifs are still there, but not in one large string.

2. Add the tidy up and trace code before each return statement and also at
the end. This keeps the nesting down, but does cause a lot of repetion
within the method.

This is fine, but you generally should have one return point in a procedure,
like:

try
{
// Assume other actions here that might "blow up"

if (x=y)
returnString = "x=y";
else
returnString = "x!=y";
}
finally
{
//DISPOSE
}

return returnString;

While this example is oversimplified, it has only one return point, which
means you can run clean up in one finally

3. Use the evil(ish) goto! That is, simply add a goto TidyUpStuff and put
all the tidying up and trace code after the TidyUpStuff label which would be
at the end of the method. I'm not an anti-goto Zealot, but I haven't used
goto since the early days of basic and would rather not revisit it if I
don't have to.

No! No! No!

Okay, there might be a case, but goto generally means you have not thought
the problem through.

Here is my two cents:

1. Procedures should complete a single action. If you find yourself writing
really long procedures, you probably have actions that can be broken out,
possibly genericized and then reused. If you find that you have written a
very similar block of code, over and over, you need a new procedure.

2. You should check input values if the full range of values are not
acceptable. For example, if you are using an int type and only allow ints
from 1-1000, you should capture values outside of this range and throw an
exception.

3. Consider output checking. This is a fuzzier area. If you know the output
value can only be between 1-1000, you should check and log when it is
outside the range (and most likely throw an exception).

4. Only handle exceptions that you either a) need cleanup on or b) you can
handle or c) you can add more clarity. Catch and release is a great idea for
fishermen, not developers. If you catch an error, you should either have a
method where you can solve at least some of the errors (ie, the error says
directory does not exist, so I will create the directory and try again) or
add clarity (Divide by zero becomes "You cannot enter 0 for {variable
name}.")

5. Try ... catch ... blocks should not be used on known quantities, or
"Paranoid programming is very expensive". Why spend cycles on something that
will not fail? The exception is the UI layer, where you want to stop users
from seeing errors. At this level, you should have some form of logging to
track down problems when the help desk is called.


--
Gregory A. Beamer
MVP; MCP: +I, SE, SD, DBA

**********************************************************************
Think Outside the Box!
**********************************************************************

 

[Nicholas Paldino [.NET/C# MVP]]

Mark,

See inline:

Apologies for the lengthy posting....
In a previous posting:
http://groups.google.com/groups?hl=...%240%2422605%24cc9e4d1f%40news.dial.pipex.com

I asked a question about how to ensure that some "tidying up" code would be
run at the end of a Method independently of where the method exits. The
consensus was to use try-finally blocks and using blocks: In the former
case, I could put all my "tidying up" code in the finally block and
everything else in the try block. In the case of simply disposing, I could
initialise in a "using" block.

Whilst this mostly met the requirements, I ran into two issues:

1. Putting large amounts of code in the try block is OK and any return
statements therein do indeed cause the code in the finally block to run.
However, the presence of the try block causes any code that would normally
generate an unhandled exception (invariably some buggy code that I've
written!) to keep quiet. In general, I like to handle only the exceptions
that I choose. The rationale being that any other kind of exception I'd
rather know about and fix the bug, which it invariably is. So, wrapping up
great chunks of code into a try block seems to be out of the question.

This isn't necessarily true. The following syntax is completely
acceptable:

try
{
// Do something.
}
finally
{
// Clean up.
}

This will have the effect of allowing the exception to bubble up, and
still execute your cleanup code. The exception only gets swallowed if you
choose to do so. If you need to catch specific exceptions, then you can use
catch statements to do that. Also, you can embed try statements within
other try statements.

2. Of course, the using block only disposes of objects that are initialised
in the using statement. However, an example of something I want to happen
just before the Method exists is writing to a TraceListener something like
"Method is now exiting...".

This isn't true either. You can do the following:

// Declare something that is disposable.
IDisposable pobjDisposable = new DisposableResource();

// Use it here.
using (pobjDisposable)
{
// Do something with pobjDisposable).
}

This allows you to initialize the object outside of the using statement,
then have it disposed of in a using statement.

Given these two points, it should give you a nice solution which will
allow you avoid everything that you outlined below.

Hope this helps.

 

[100]

Hi Mark,

1. Putting large amounts of code in the try block is OK and any return
statements therein do indeed cause the code in the finally block to run.
However, the presence of the try block causes any code that would normally
generate an unhandled exception (invariably some buggy code that I've
written!) to keep quiet. In general, I like to handle only the exceptions
that I choose. The rationale being that any other kind of exception I'd
rather know about and fix the bug, which it invariably is. So, wrapping up
great chunks of code into a try block seems to be out of the question.

Finally blocks doesn't catch any exceptions. That is if you enter finally
block because of exception the exception will be passed down to the caller.

2. Of course, the using block only disposes of objects that are initialised
in the using statement. However, an example of something I want to happen
just before the Method exists is writing to a TraceListener something like
"Method is now exiting...".

Actually any *using* statement generates try-finally block. So, if
try-finally doesn't work for you *using* shouldn't work as well.

I don't see any problem why not to use "finally". Maybe you catch the
exceptions and eat them up somewhere down in the calling chain.
Anyways, if you catch an exception you will have your finally blocks
executed and then the catch block. If you have unhandled exception, though,
you will get first a notification for the unhandled exception and then the
finally blocks will be executed.

Can you post some example where a finally block swallows unhandled
exceptions?

B\rgds
100

 

[Mark]

Thanks 100, Nicholas Paldino and Cowboy for you responses. See inline...

Hi Mark,
[snip]

Can you post some example where a finally block swallows unhandled
exceptions?

Certainly. I think that, as ever, I haven't framed my question very well!
The exception handling isn't really the issue as I'm confortable with
try-catch-finally clauses. My real problem is to do with flow control in
general - the problem is highlighted by exception handling, but it doesn't
have to be. Here's an example (once again, apologies for the length - also,
I'm not pasting this in so there may be the odd typo)...

private void MyMethod()
{
MyIDisposableClass midc = new MyIDisposableClass();
// some code;
try
{
// some code that might throw and exception;
}
catch
{
// do the tidying up here
midc.Dispose();
Trace.WriteLine("Happy Hogmanay!");
Trace.WriteLine("MyMethod is bombing out here.");
return; // exit point 1
}

if(somevalue)
{
// do something if the if is true;
}
else
{
// we wish to exit the Method here
midc.Dispose();
Trace.WriteLine("Happy Hogmanay!");
Trace.WriteLine("MyMethod is bombing out here.");
return; // exit point 2
}

// some more ordinary code;
try
{
// some more code that might throw and exception;
}
catch
{
// again, do the tidying up here
midc.Dispose();
Trace.WriteLine("Happy Hogmanay!");
Trace.WriteLine("MyMethod is bombing out here.");
return; // exit point 3
}

if(someothervalue)
{
// do something if the if is true;
}
else
{
// we wish to exit the Method here for some reason
midc.Dispose();
Trace.WriteLine("Happy Hogmanay!");
Trace.WriteLine("MyMethod is bombing out here.");
return; // exit point 4
}

// final bits of code here, then it's time to tidy up
midc.Dispose();
Trace.WriteLine("Happy Hogmanay!");
Trace.WriteLine("MyMethod is bombing out here.");
// exit point 5
}

As you can see, there is much repition of code here - five time in fact in
this example: 2 tray-catch blocks, 2 if statement and the method itself.
This makes maintence difficult. Also, there are 5 places that the method can
exit, which again in general doesn't make for easily maintained code. What
would be nice is to have the Dispose and two Trace lines appear once and no
mid-method returns. One way is simply to nest the try-catchs and the ifs,
but the nesting can get very extensive, particularly when there are in fact
10 possible exit points! As I mentioned, a goto in the place of each exit
point would do the trick but I really do want to avoid doing that. There
must be a "standard" way of doing this, but I'm not sure what it is.

Any help would really be appreciated. BTW, don't take the "tidy up" code
literally, I've just used these three lines as an example. In practice, it
could consist of all manner of bits and pieces!

Cheers
Mark

 

[100]

Hi Mark,

Why don't you just make something like this?

private void MyMethod()
{
try
{
using(MyIDisposableClass midc = new MyIDisposableClass())
{
// some code;
// some code that might throw an exception;
if(somevalue)
{
do something if the if is true;
}
else
{
throw new MyException(....)
}
// some more ordinary code;
if(someothervalue)
{
do something if the if is true;
}
else
{
throw new MyException(....)
}

}
}
catch(ExceptionType1 e)
{
//Emits some lines to Trace
}
catch(ExceptionType2 e)
{
//Emits some lines to Trace
}
catch(MyException e)
{
//Emits some lines to Trace
}
finally
{
//Do some more clean up here
}
}
For if statements you can use your own type of exception, which you can
define like
class MyException: ApplicationException
{
.....
}

If you don't care about the type of exceptions you want to swallow just
leave the catch bodies empty and move the emit code in the finally block.
If you don't want to swallow an exception at all but only report it
put a single throw operation at the and of the boddy. This will rethrow the
exception
try
{
...
}
catch(ExceptionType1 e)
{
//Log exception
throw;
}

if you don't really care about the type of exception you have and just want
to make a record in the log file without consuming any exception you can
write your code like this

try
{
using(MyIDisposableClass midc = new MyIDisposableClass())
{
// some code;
// some code that might throw an exception;
if(somevalue)
{
do something if the if is true;
}
else
{
throw new ApplicationException(....)
}
// some more ordinary code;
if(someothervalue)
{
do something if the if is true;
}
else
{
throw new ApplicationException(....)
}

}
}
catch(Exception e)
{
//Emits some lines to Trace
throw;
}
finally
{
//Do some more clean up here
// if you don't have anything more to clean remove the finally block;
}

My advice is not to use try catch block of the form
try
{
}
catch
{
}
this will catch any type of exceptions even "unmanaged" ones
If you want to distingush different types of exception use multiple catch
blocks and beaware that the order of the catch blocks does matter. Put the
more specific ones on the top and the more general ones on the bottom.

HTH
B\rgds
100

Thanks 100, Nicholas Paldino and Cowboy for you responses. See inline...

Hi Mark,
[snip]

Can you post some example where a finally block swallows unhandled
exceptions?

Certainly. I think that, as ever, I haven't framed my question very well!
The exception handling isn't really the issue as I'm confortable with
try-catch-finally clauses. My real problem is to do with flow control in
general - the problem is highlighted by exception handling, but it doesn't
have to be. Here's an example (once again, apologies for the length - also,
I'm not pasting this in so there may be the odd typo)...

private void MyMethod()
{
MyIDisposableClass midc = new MyIDisposableClass();
// some code;
try
{
// some code that might throw and exception;
}
catch
{
// do the tidying up here
midc.Dispose();
Trace.WriteLine("Happy Hogmanay!");
Trace.WriteLine("MyMethod is bombing out here.");
return; // exit point 1
}

if(somevalue)
{
// do something if the if is true;
}
else
{
// we wish to exit the Method here
midc.Dispose();
Trace.WriteLine("Happy Hogmanay!");
Trace.WriteLine("MyMethod is bombing out here.");
return; // exit point 2
}

// some more ordinary code;
try
{
// some more code that might throw and exception;
}
catch
{
// again, do the tidying up here
midc.Dispose();
Trace.WriteLine("Happy Hogmanay!");
Trace.WriteLine("MyMethod is bombing out here.");
return; // exit point 3
}

if(someothervalue)
{
// do something if the if is true;
}
else
{
// we wish to exit the Method here for some reason
midc.Dispose();
Trace.WriteLine("Happy Hogmanay!");
Trace.WriteLine("MyMethod is bombing out here.");
return; // exit point 4
}

// final bits of code here, then it's time to tidy up
midc.Dispose();
Trace.WriteLine("Happy Hogmanay!");
Trace.WriteLine("MyMethod is bombing out here.");
// exit point 5
}

As you can see, there is much repition of code here - five time in fact in
this example: 2 tray-catch blocks, 2 if statement and the method itself.
This makes maintence difficult. Also, there are 5 places that the method can
exit, which again in general doesn't make for easily maintained code. What
would be nice is to have the Dispose and two Trace lines appear once and no
mid-method returns. One way is simply to nest the try-catchs and the ifs,
but the nesting can get very extensive, particularly when there are in fact
10 possible exit points! As I mentioned, a goto in the place of each exit
point would do the trick but I really do want to avoid doing that. There
must be a "standard" way of doing this, but I'm not sure what it is.

Any help would really be appreciated. BTW, don't take the "tidy up" code
literally, I've just used these three lines as an example. In practice, it
could consist of all manner of bits and pieces!

Cheers
Mark

 

[Mark]

Hi Mark,

Why don't you just make something like this?

Fantastic response! I do really appreciate this. See inline...

private void MyMethod()
{
try
{
using(MyIDisposableClass midc = new MyIDisposableClass())
{
// some code;
// some code that might throw an exception;
if(somevalue)
{
do something if the if is true;
}
else
{
throw new MyException(....)
}
// some more ordinary code;
if(someothervalue)
{
do something if the if is true;
}
else
{
throw new MyException(....)
}

}
}
catch(ExceptionType1 e)
{
//Emits some lines to Trace
}
catch(ExceptionType2 e)
{
//Emits some lines to Trace
}
catch(MyException e)
{
//Emits some lines to Trace
}
finally
{
//Do some more clean up here
}
}

I like this idea, but how do I know what threw the exception? E.g.,
supposing ExceptionType1 is a COMException, and I have 2 or 3 places in the
try block, each of which can throw a COMException (assuming I know about 1
of them but have written a bug into the code for the other 2)?

For if statements you can use your own type of exception, which you can
define like
class MyException: ApplicationException
{
.....
}

That makes sense. I hadn't considered writing my own exceptions.

If you don't care about the type of exceptions you want to swallow just
leave the catch bodies empty and move the emit code in the finally block.
If you don't want to swallow an exception at all but only report it
put a single throw operation at the and of the boddy. This will rethrow the
exception
try
{
...
}
catch(ExceptionType1 e)
{
//Log exception
throw;
}

I like this idea a lot, as many of the exceptions I wish to log and then
bail out and I was finding that the try block was simply swallowing the
exceptions that I wasn't aware could have occurred, and leaving me none the
wiser! Thanks.

if you don't really care about the type of exception you have and just want
to make a record in the log file without consuming any exception you can
write your code like this

try
{
using(MyIDisposableClass midc = new MyIDisposableClass())
{
// some code;
// some code that might throw an exception;
if(somevalue)
{
do something if the if is true;
}
else
{
throw new ApplicationException(....)
}
// some more ordinary code;
if(someothervalue)
{
do something if the if is true;
}
else
{
throw new ApplicationException(....)
}

}
}
catch(Exception e)
{
//Emits some lines to Trace
throw;
}
finally
{
//Do some more clean up here
// if you don't have anything more to clean remove the finally block;
}

My advice is not to use try catch block of the form
try
{
}
catch
{
}
this will catch any type of exceptions even "unmanaged" ones

In general, I do try to catch only those exceptions that I wish to manage.
Poor example on my part.

If you want to distingush different types of exception use multiple catch
blocks and beaware that the order of the catch blocks does matter. Put the
more specific ones on the top and the more general ones on the bottom.

HTH

Too right it does! Many thanks
Mark

B\rgds
100

[snip]

 

[Hung Jung Lu]

As you can see, there is much repition of code here - five time in fact in
this example: 2 tray-catch blocks, 2 if statement and the method itself.
This makes maintence difficult. Also, there are 5 places that the method can
exit, which again in general doesn't make for easily maintained code. What
would be nice is to have the Dispose and two Trace lines appear once and no
mid-method returns.

(a) Goto statements are not necessarily evil.

(b) I am not sure whether others have posted similar solution, but you
can emulate a goto by throwing exceptions. Warning: many programmers
are aghast to this idea. But they usually are unaware that this is the
standard approach in at least another language: Python. (More to the
point, C# designers seem to have taken into account Python's exception
handling approach, and have discarded Java's checked exceptions. A few
features in C# actually closely resemble Python. I have read an
article on C# exception design that explicitly mentioned Python. All
this is just to say that this approach is not coming from nowhere.
Many people don't like it, but others do. I am aware of the
controversies.)

public class Jump: Exception {}
....
private void MyMethod()
{
try
{
...
throw new Jump();
...
throw new Jump();
...
throw new Jump();
...
}
catch (Jump) {}
finally
{
midc.Dispose();
Trace.WriteLine("Happy Hogmanay!");
Trace.WriteLine("MyMethod is bombing out here.");
}
}

regards,

Hung Jung

 

[100]

Hi Mark,

See at the bottom.

Fantastic response! I do really appreciate this. See inline...


I like this idea, but how do I know what threw the exception? E.g.,
supposing ExceptionType1 is a COMException, and I have 2 or 3 places in the
try block, each of which can throw a COMException (assuming I know about 1
of them but have written a bug into the code for the other 2)?

I can give you two ideas for workarround:

1. Use local variable to bookmark the call that throws the exception
private void MyMethod()
{
int bookmark = -1;
try
{
using(MyIDisposableClass midc = new MyIDisposableClass())
{
bookmark = 0;
// some code that throw COMException exception;
bookmark = 1;
// some code that might throw other COMException exception;
if(somevalue)
{
do something if the if is true;
}
else
{
throw new MyException(....)
}
// some more code that can throw COMException exception
bookmark =2;
if(someothervalue)
{
do something if the if is true;
}
else
{
throw new MyException(....)
}

}
}
catch(COMException)
{
switch(bookmark)
{
case0:
//Log for first call
break;
case1:
//Log for second call
break;
case2:
//Log for third call
break;
}
throw
}
catch(ExceptionType2 e)
{
//Emits some lines to Trace
}
catch(MyException e)
{
//Emits some lines to Trace
}
finally
{
//Do some more clean up here
}
}

2. Chaining COMException exceptions

private void MyMethod()
{
try
{
using(MyIDisposableClass midc = new MyIDisposableClass())
{

try
{
// some code that throw COMException exception;
}
catch(COMException e)
{
throw new COMException("Log message 1", e);
}

try
{
// some code that might throw other COMException exception;
}
catch(COMException e)
{
throw new COMException("Log message 2", e);
}
if(somevalue)
{
do something if the if is true;
}
else
{
throw new MyException(....)
}

try
{
// some more code that can throw COMException exception
}
catch(COMException e)
{
throw new COMException("Log message 3", e);
}
if(someothervalue)
{
do something if the if is true;
}
else
{
throw new MyException(....)
}

}
}
catch(COMException e)
{
EmitLogLine(e.Message);
throw e.InnerException;
}
catch(...)
.....
finally
{
...
}

I'm sure there are better solutions.

HTH
B\rgds
100

 

[Mark]

"Mark" <[email protected]> wrote in message

(a) Goto statements are not necessarily evil.

Indeed. See my comment below...

(b) I am not sure whether others have posted similar solution, but you
can emulate a goto by throwing exceptions. Warning: many programmers
are aghast to this idea. But they usually are unaware that this is the
standard approach in at least another language: Python. (More to the
point, C# designers seem to have taken into account Python's exception
handling approach, and have discarded Java's checked exceptions. A few
features in C# actually closely resemble Python. I have read an
article on C# exception design that explicitly mentioned Python. All
this is just to say that this approach is not coming from nowhere.
Many people don't like it, but others do. I am aware of the
controversies.)

public class Jump: Exception {}
...
private void MyMethod()
{
try
{
...
throw new Jump();
...
throw new Jump();
...
throw new Jump();
...
}
catch (Jump) {}
finally
{
midc.Dispose();
Trace.WriteLine("Happy Hogmanay!");
Trace.WriteLine("MyMethod is bombing out here.");
}
}

Yes, have played extensively with various solutions, it seems to me that
this is a goto by a different name? That is, exception handling seems to be
intrinsically unstructured in that by manually throwing an exception, you
are effectively causing a jump to a catch block. What I've used is something
like this (please excuse any typos):

MyMethod()
{
// initialise classes here

try
{
// the main block of code which could generate unexpected exceptions

try
{
// some code that i think might generate COMexceptions
}
catch(COMException ex)
{
Trace.Writeline(ex.Message);
throw new ExitException(); // This is just a glorified goto simply
designed to ensure that the following code doesn't get run!
}

if(some_test)
{
// all is well
}
else
{
// another disaster, so...
throw new ExitException(); // The glorified goto again!
}

// more big chunks of code in this method
}

catch(Exception ex)
{
if(ex.GetType().name != "ExitException")
{
// We didn't get here from my glorified goto, so it must be one of
those unexpected exceptions;
throw; // i'd rather unexpected exceptions caused the method to
blow up
}
}

finally
{
// tidy up - provided i use my glorified goto, this always gets run
Trace.Writeline("Exiting method");
}
}
ExitExcption is simply a class that does nothing except inherit
System.Exception and has a unique name.

I'm happy that the about meets the requirements of:
- No gotos
- One exit point
- The finally always gets run

The thing that strikes me about this solution is that i'm creating a custom
exception handler with the sole purpose of acting as a goto. From what I've
read, exception handling is also relatively slow compared to "normal" flow
control. So, I've also tried this:

MyMethod()
{
// initialise classes here

// the main block of code which could generate unexpected exceptions

try
{
// some code that i think might generate COMexceptions
}
catch(COMException ex)
{
Trace.Writeline(ex.Message);
goto TidyUpAndExit;
}

if(some_test)
{
// all is well
}
else
{
// another disaster, so...
goto TidyUpAndExit;
}

// more big chunks of code in this method
}


TidyUpAndExit:
// tidy up - provided i use my glorified goto, this always gets run
Trace.Writeline("Exiting method");
}

In terms of readability, IMHO, the latter is more readable (mainly because
the goto label says exactly what it does and it's the only one in the
method). It's also more compact with less nesting and doesn't involve any
custom error handlers. However, some would say that it's the devil's own
code because it uses the evil "goto". I'd be interested in people's
thoughts?

Thanks for all your help
Mark

 

[Mark]

Hi - thanks! See bottom of message...

Hi Mark,

See at the bottom.
[snip]

I can give you two ideas for workarround:

1. Use local variable to bookmark the call that throws the exception
private void MyMethod()
{
int bookmark = -1;
try
{
using(MyIDisposableClass midc = new MyIDisposableClass())
{
bookmark = 0;
// some code that throw COMException exception;
bookmark = 1;
// some code that might throw other COMException exception;
if(somevalue)
{
do something if the if is true;
}
else
{
throw new MyException(....)
}
// some more code that can throw COMException exception
bookmark =2;
if(someothervalue)
{
do something if the if is true;
}
else
{
throw new MyException(....)
}

}
}
catch(COMException)
{
switch(bookmark)
{
case0:
//Log for first call
break;
case1:
//Log for second call
break;
case2:
//Log for third call
break;
}
throw
}
catch(ExceptionType2 e)
{
//Emits some lines to Trace
}
catch(MyException e)
{
//Emits some lines to Trace
}
finally
{
//Do some more clean up here
}
}

2. Chaining COMException exceptions

private void MyMethod()
{
try
{
using(MyIDisposableClass midc = new MyIDisposableClass())
{

try
{
// some code that throw COMException exception;
}
catch(COMException e)
{
throw new COMException("Log message 1", e);
}

try
{
// some code that might throw other COMException exception;
}
catch(COMException e)
{
throw new COMException("Log message 2", e);
}
if(somevalue)
{
do something if the if is true;
}
else
{
throw new MyException(....)
}

try
{
// some more code that can throw COMException exception
}
catch(COMException e)
{
throw new COMException("Log message 3", e);
}
if(someothervalue)
{
do something if the if is true;
}
else
{
throw new MyException(....)
}

}
}
catch(COMException e)
{
EmitLogLine(e.Message);
throw e.InnerException;
}
catch(...)
.....
finally
{
...
}

I'm sure there are better solutions.

Nice ideas - thanks. I've also tried a couple of other solutions to this
problem, both of which use a try-catch block whenever i want to handle an
exception. That is, using a separate try-catch block for each exception, so
that i don't need to use chained exceptiions or labels (though I do like the
chained exceptions!). I've actually listed the code in reply to Hung Jung
Lu's posting. Basically, it shows how i'm using a custom error handler to
notify the outermost catch block whether the exception is "unexpected" or
has been handled. really, it's just a fudge to force the outermost finally
block to run! As I imply in that message, the lengths that one has to go to
to avoid using goto seem to me IMHO to make the code harder to read and
maintain than if a well-named single goto label were used. I'm actually
probably going to use goto therefore. Yikes! Oh dear, I think I've just
opened myself up for some serious flames...

Cheers
Mark

 

[100]

Hi Mark,
Don't be so hard on yourself when using *goto*. If you believe that it will
make your code clearer just go ahead and use it. *Goto* has lived though all
these years because it indeed makes some time the code shorter and easier to
maintain (even modern languages like C# don't dare to get rid of it. ). If
you keep your methods short and readable *goto* won't hurt your programs.

I'm not *goto* advocate and frankly I avoid using it, but I won't hesitate
to go with *goto* if I need to.

B\rgds
100

Hi - thanks! See bottom of message...

Hi Mark,

See at the bottom.
[snip]

I can give you two ideas for workarround:

1. Use local variable to bookmark the call that throws the exception
private void MyMethod()
{
int bookmark = -1;
try
{
using(MyIDisposableClass midc = new MyIDisposableClass())
{
bookmark = 0;
// some code that throw COMException exception;
bookmark = 1;
// some code that might throw other COMException exception;
if(somevalue)
{
do something if the if is true;
}
else
{
throw new MyException(....)
}
// some more code that can throw COMException exception
bookmark =2;
if(someothervalue)
{
do something if the if is true;
}
else
{
throw new MyException(....)
}

}
}
catch(COMException)
{
switch(bookmark)
{
case0:
//Log for first call
break;
case1:
//Log for second call
break;
case2:
//Log for third call
break;
}
throw
}
catch(ExceptionType2 e)
{
//Emits some lines to Trace
}
catch(MyException e)
{
//Emits some lines to Trace
}
finally
{
//Do some more clean up here
}
}

2. Chaining COMException exceptions

private void MyMethod()
{
try
{
using(MyIDisposableClass midc = new MyIDisposableClass())
{

try
{
// some code that throw COMException exception;
}
catch(COMException e)
{
throw new COMException("Log message 1", e);
}

try
{
// some code that might throw other COMException exception;
}
catch(COMException e)
{
throw new COMException("Log message 2", e);
}
if(somevalue)
{
do something if the if is true;
}
else
{
throw new MyException(....)
}

try
{
// some more code that can throw COMException exception
}
catch(COMException e)
{
throw new COMException("Log message 3", e);
}
if(someothervalue)
{
do something if the if is true;
}
else
{
throw new MyException(....)
}

}
}
catch(COMException e)
{
EmitLogLine(e.Message);
throw e.InnerException;
}
catch(...)
.....
finally
{
...
}

I'm sure there are better solutions.

Nice ideas - thanks. I've also tried a couple of other solutions to this
problem, both of which use a try-catch block whenever i want to handle an
exception. That is, using a separate try-catch block for each exception, so
that i don't need to use chained exceptiions or labels (though I do like the
chained exceptions!). I've actually listed the code in reply to Hung Jung
Lu's posting. Basically, it shows how i'm using a custom error handler to
notify the outermost catch block whether the exception is "unexpected" or
has been handled. really, it's just a fudge to force the outermost finally
block to run! As I imply in that message, the lengths that one has to go to
to avoid using goto seem to me IMHO to make the code harder to read and
maintain than if a well-named single goto label were used. I'm actually
probably going to use goto therefore. Yikes! Oh dear, I think I've just
opened myself up for some serious flames...

Cheers
Mark

 

[Dave]

In terms of readability, IMHO, the latter is more readable (mainly because
the goto label says exactly what it does and it's the only one in the
method). It's also more compact with less nesting and doesn't involve any
custom error handlers. However, some would say that it's the devil's own
code because it uses the evil "goto". I'd be interested in people's
thoughts?

Thanks for all your help
Mark

I like where you are going with this but I must confess that both mechanisms
make me cringe; the problem with throwing exceptions as a form of flow
control is that it will perform extremely poorly.

Each time you throw an exception it causes the underlying exception handling
(SEH) mechanism to begin a 2 phase process of searching the call stack for
activation records that contain a matching filter, and when one is found it
then begins the 2nd phase where it looks for finally blocks to run (this is
the simple description). When the initial exception is raised it goes
through the kernel's trap manager, where it is packaged up in a standard OS
SEH form, then it notifies attached debuggers, then it begins the 2 phase
stack walk. This is by definition non-local and will perform extremely
badly. It is a long ways from setjmp/longjmp.

You may not care about the performance in your case since this may be a
manual operation anyway but I would advocate against this as a general
technique. I would especially not use it if the exception crosses remoting
boundaries or if the thread wanders in-and-out of managed/unmanaged code.

Even though the much maligned goto has a bad rep (well deserved) it does
have one advantage over using exceptions as a form of flow control in that
it is local and does not have the overhead of SEH.

I like the other alternative mentioned elsewhere on this thread about
maintaining a state variable that keeps track of where you are at in the
routine so that the catch or finally handler can tell what it was doing when
an exception was thrown. This has one advantage over the others....since an
exception can occur at any time (e.g. ThreadAbortException) you can always
tell exactly where you were at even if you didn't throw the exception
yourself. What I don't like about this is that it manually adds bookkeeping
information that we really shouldn't have to (the compiler should keep track
of this), and this adds to the complexity and makes the code more error
prone.

Dave