stuck with return value

[Guest]

Hi,

I have a function that adds data to a database and I want to return true if
the database was updated and false if there was a problem, so I am using a
try... catch block...

My problem is that I can return true ok is the routine succeeds but where do
I put the return false; statement id the process fails - I get a compile
error say unreachable code with whatever i try...

Code snippet:

public bool Add()
{
try
{
// Do stuff
return true;
}
catch (Exception ex)
{
throw new Exception ("Unable to update database " + ex.Message)
}
}

If i put return false before the throw then the throw is unreachable, if I
put it after then the return is unreachable.... what is the proper way to do
this??

Thanks in advance

 

[Wiktor Zychla]

public bool Add()

{
try
{
// Do stuff
return true;
}
catch (Exception ex)
{
throw new Exception ("Unable to update database " + ex.Message)
}
}

you have to decide if you return false or throw an exception. you cannot do
both. in your code the high-level method will have to catch the exception
(because of throw) and thus will not be interested in returned value.

void HighLevelMethod()
{
try
{
bool res = Add();
}
catch
{
// if you throw the exception inside Add() then the execution will
// continue here with no value returned from Add()
}
}

Wiktor Zychla

 

[Guest]

Thanks,

that makes sense

you have to decide if you return false or throw an exception. you cannot do
both. in your code the high-level method will have to catch the exception
(because of throw) and thus will not be interested in returned value.

void HighLevelMethod()
{
try
{
bool res = Add();
}
catch
{
// if you throw the exception inside Add() then the execution will
// continue here with no value returned from Add()
}
}

Wiktor Zychla

 

[Dan Bass]

Depends on what you're doing regarding exception handling...

If you want to pass your exceptions up to the higher level and handle the
errors there (generally an accepted rule of thumb of OOP), then make Add
void and not bool. If an error occurs, then the try/catch blocks in the
higher code will catch and deal with it.

You need to look at the code and decide what you want to happen should the
Add fail...
For example, if it makes no difference to what you're doing and it's not a
major problem, then log the problem as a warning (wrap the code as you have
done, but log it rather than rethrowing the error) and carry on.
If it is a major error and your application cannot proceed without it, then
you should catch the error higher up, log it if possible (whether to disk,
to database or message to the user), and shut the application down.

Another thing to consider which will play a part here is whether there is
anything that needs "shutting" down before leaving the function. I suspect
you'll be opening a database connection, which you'll want to close...
Do this like this inside your Add():

SqlConnection myConnection = new SqlConnection();
try
{
// open connection and do your stuff here
}
catch ( Exception ex )
{
throw ( ex );
}
finally
{
if ( myConnection.State == ConnectionState.Open )
{
myConnection.Close();
}
}

Finally, remember that you may want to catch certain kinds of exceptions...
For example, if the connection fails to be established (SqlException is
thrown), you could do something different in your exception handling than if
a null object was referenced...

 

[Joakim Karlsson]

Another thing to consider which will play a part here is whether there is
anything that needs "shutting" down before leaving the function. I suspect
you'll be opening a database connection, which you'll want to close...
Do this like this inside your Add():

SqlConnection myConnection = new SqlConnection();
try
{
// open connection and do your stuff here
}
catch ( Exception ex )
{
throw ( ex );
}
finally
{
if ( myConnection.State == ConnectionState.Open )
{
myConnection.Close();
}
}

If you, as in the example above, does not do anything in the exception
handler, then just skip adding the catch statement at all. The exception
will get propagated up the stack automatically.

If you do want to perform some handling in response to a certain kind of
exception, then perform the operation you want and rethrow the
exception. If you need to rethrow the same exception, rethrow it like this:

catch(SqlException ex)
{
// Do something with ex

throw; // Do not use throw(ex);
}

as this will keep the stack trace in the exception object from being
rewritten.

/Joakim

 

[Dan Bass]

Joakim,

Excuse my ignorance on this, "throw(ex);" will overwrite the stacktrace
information how?
How would this compare to an exception caught from just doing a "throw;" ?

Thanks.

Dan.

 

[=?ISO-8859-1?Q?=22Anders_Nor=E5s_=5BMCAD=5D=22?=]

Excuse my ignorance on this, "throw(ex);" will overwrite the stacktrace
information how?
How would this compare to an exception caught from just doing a "throw;" ?

In C# throw(ex) rethrows the exception and overrides the original stack
trace changing the origin of the exception to the point where the
exception was rethrown.

public class MyClass {
public static void ThrowNotImplementedException() {
throw new NotImplementedException("Not implemented"); // <-- Line 6
}

public static void Main(){
try {
ThrowNotImplementedException();
} catch (Exception ex) {
Console.WriteLine("Exception: {0}",ex.GetType());
throw(ex); // <-- Line 18
}
}
}

Yields the following output:
Exception: System.NotImplementedException

Unhandled Exception: System.NotImplementedException: Not implemented
at MyClass.Main() in c:\temp\defualt.cs:line 18

If you rethrow the exception by just using the throw keyword the stack
trace is persisted.

Exception: System.NotImplementedException

Unhandled Exception: System.NotImplementedException: Not implemented
at MyClass.ThrowNotImplementedException() in c:\temp\defualt.cs:line 6
at MyClass.Main() in c:\temp\defualt.cs:line 18

MSIL has two instructions for throwing exceptions; throw and rethrow.
The C# instructions throw(ex) and throw ex compiles to MSILs throw
instruction. The C# throw instruction without parameters compiles to
MSILs rethrow instruction.

Anders Norås
http://dotnetjunkies.com/weblog/anoras/

 

[Dan Bass]

Cool. Thanks!

Excuse my ignorance on this, "throw(ex);" will overwrite the stacktrace
information how?
How would this compare to an exception caught from just doing a "throw;"
?

In C# throw(ex) rethrows the exception and overrides the original stack
trace changing the origin of the exception to the point where the
exception was rethrown.

public class MyClass {
public static void ThrowNotImplementedException() {
throw new NotImplementedException("Not implemented"); // <-- Line
6
}

public static void Main(){
try {
ThrowNotImplementedException();
} catch (Exception ex) {
Console.WriteLine("Exception: {0}",ex.GetType());
throw(ex); // <-- Line 18
}
}
}

Yields the following output:
Exception: System.NotImplementedException

Unhandled Exception: System.NotImplementedException: Not implemented
at MyClass.Main() in c:\temp\defualt.cs:line 18

If you rethrow the exception by just using the throw keyword the stack
trace is persisted.

Exception: System.NotImplementedException

Unhandled Exception: System.NotImplementedException: Not implemented
at MyClass.ThrowNotImplementedException() in c:\temp\defualt.cs:line 6
at MyClass.Main() in c:\temp\defualt.cs:line 18

MSIL has two instructions for throwing exceptions; throw and rethrow. The
C# instructions throw(ex) and throw ex compiles to MSILs throw
instruction. The C# throw instruction without parameters compiles to MSILs
rethrow instruction.

Anders Norås
http://dotnetjunkies.com/weblog/anoras/

 

[Joakim Karlsson]

Excuse my ignorance on this, "throw(ex);" will overwrite the stacktrace
information how?
How would this compare to an exception caught from just doing a "throw;" ?

Consider the code below:

If One() uses throw;, the output will be:

at Class1.Two() in class1.cs:line 32
at Class1.One() in class1.cs:line 26
at Class1.Main(String[] args) in class1.cs:line 9

whereas if you use throw(ex);, the output will be:

at Class1.One() in class1.cs:line 25
at Class1.Main(String[] args) in class1.cs:line 9

In the throw(ex) case, the exception handlers furhter up the call stack
will consider it being originally thrown from where it is actually rethrown.

/Joakim

using System;

class Class1
{
static void Main(string[] args)
{
try
{
One();
}
catch(Exception e)
{
Console.WriteLine(e.StackTrace);
}
}

static void One()
{
try
{
Two();
}
catch(Exception e)
{
throw e;
//throw;
}
}

static void Two()
{
throw new Exception();
}
}

 

[Cor Ligthert]

Hp,

Here a sample to show you the use of more catches of different exceptions.

This one is a little bit else than the normal one I use because I have the
construct of the connection normally in it and use dispose than instead of
close.
\\\
try
{
conn.Open();
if (ds.HasChanges())
{da.Update(ds);}
}
catch (DBConcurrencyException ex)
{HandleConcurrencyErrors(ex);}
catch (Exception ex)
{HandleNormalErrors(ex);}
finally
{conn.Close();}
///
I hope this helps,

Cor

 

[Jon Skeet [C# MVP]]

I have a function that adds data to a database and I want to return true if
the database was updated and false if there was a problem, so I am using a
try... catch block...

My problem is that I can return true ok is the routine succeeds but where do
I put the return false; statement id the process fails - I get a compile
error say unreachable code with whatever i try...

Code snippet:

public bool Add()
{
try
{
// Do stuff
return true;
}
catch (Exception ex)
{
throw new Exception ("Unable to update database " + ex.Message)
}
}

The code you've posted compiles (once the semi-colon has been added).

Could you post a short but complete program which demonstrates the
problem?

See http://www.pobox.com/~skeet/csharp/complete.html for details of
what I mean by that.

 

[Joakim Karlsson]

The code you've posted compiles (once the semi-colon has been added).

Could you post a short but complete program which demonstrates the
problem?

See http://www.pobox.com/~skeet/csharp/complete.html for details of
what I mean by that.

The problem is that the original poster wanted to put a 'return false;'
statement inside the exception handler in addition to throwing an
exception. This will not, for obvious reasons, compile.

/Joakim

 

[Joakim Karlsson]

+1 on your article. It is so much easier to determine what is wrong with
a code snippet if you can just copy the entire thing into a fresh
project, compile it, and try it out.

/Joakim

 

[Jon Skeet [C# MVP]]

The problem is that the original poster wanted to put a 'return false;'
statement inside the exception handler in addition to throwing an
exception. This will not, for obvious reasons, compile.

No - but the stated *reason* he wanted to put a 'return false;'
statement inside the exception handler was that otherwise it wouldn't
compile, whereas actually it does.

If we saw the actual code which wasn't compiling in the first place, we
could say where he *could* put a 'return false;'.

 

[Joakim Karlsson]

No - but the stated *reason* he wanted to put a 'return false;'
statement inside the exception handler was that otherwise it wouldn't
compile, whereas actually it does.

I interpreted it the other way around - 'OK this method is declared as
returning true or false, indicating success or failure, where do I put
the return false in case the method fails?' But English is not my native
tongue so I consider myself corrected.

/Joakim

 

[Jon Skeet [C# MVP]]

I interpreted it the other way around - 'OK this method is declared as
returning true or false, indicating success or failure, where do I put
the return false in case the method fails?' But English is not my native
tongue so I consider myself corrected.

No, you could well be right

 

[Cor Ligthert]

Joakim,

I see that I did give a sample however withouth the returns which where
asked

\\\
try
{
conn.Open();
if (ds.HasChanges())
{da.Update(ds);
return true;}
}
catch (DBConcurrencyException ex)
{HandleConcurrencyErrors(ex);
return false;}
catch (Exception ex)
{HandleNormalErrors(ex);
return false;}
finally
{conn.Close();}
///

Although because of the different exceptions that can be throwed by an
adapter is that not the right way in my opinion.

Cor

 

[Joakim Karlsson]

Joakim,

I see that I did give a sample however withouth the returns which where
asked

\\\
try
{
conn.Open();
if (ds.HasChanges())
{da.Update(ds);
return true;}
}
catch (DBConcurrencyException ex)
{HandleConcurrencyErrors(ex);
return false;}
catch (Exception ex)
{HandleNormalErrors(ex);
return false;}
finally
{conn.Close();}
///

Although because of the different exceptions that can be throwed by an
adapter is that not the right way in my opinion.

Cor

Whether to use an exception or a return value to indicate success or
failure of an operation is one of those 'it depends on the situation...'
type of things. If you have control of both the consumed class and the
consumer class and have knowledge of the situations your classes will be
used, it is much easier to decide whether to use return values or
exceptions.

If you are writing a class that will be used by others in situations not
known to you, exceptions is the way to go IMO, with all the information
they can provide.

But *per method* you have to choose one or the other. It's weird having
a return value of true meaning success and an exception meaning failure.


For some pointers, you might check out:
http://msdn.microsoft.com/library/d...l/cpconbestpracticesforhandlingexceptions.asp

/Joakim