17

If I have a method that returns something, like

public DataTable ReturnSomething()
{
   try
   {  
      //logic here
     return ds.Tables[0];
   }
   catch (Exception e)
   {
      ErrorString=e.Message;
   }
}

This produces compiler error, obviously because catch{} block does not return anything.

So when I have methods with return values I don't use try-catch block, which is a bad practice. If there is an error, I would like to set error string to that error. But then I need a return value as well. Advice?

3
  • "So when I have methods with return values I don't use try-catch block, which is a bad practice." says who? Most methods return values without being wrapped in try blocks. Mar 2, 2009 at 15:46
  • Beaten by 20 seconds! Indeed, juts what I was going to say; if there is an exception - UNLESS you expect it and can do something useful about it, let it bubble to the caller. Mar 2, 2009 at 15:47
  • 3
    Swallowing exceptions is the real problem here.
    – DK.
    Mar 2, 2009 at 16:24

12 Answers 12

36

Store your return value in a temporary variable like this:

public DataTable ReturnSomething()
{
    DataTable returnValue = null;

    try
    {
        //logic here
        returnValue = ds.Tables[0]; 
    }
    catch (Exception e)
    {
        ErrorString=e.Message;
    }

    return returnValue;
}
2
  • 4
    Like others have noted, you should think about throwing the exception instead of setting an ErrorString though. If you want an exception with your own message, you should throw a new exception, with the original exception as inner exception. throw new ArgumentException("My message", e);
    – Svish
    Mar 20, 2009 at 11:43
  • 1
    I agree with Svish and others; although I've attempted to answer your question as asked, I don't think it's the best solution to your problem.
    – Simon
    Mar 23, 2009 at 8:54
18

You should raise/throw the exception in your catch block and handle it in the calling method.

public void invokeFaultyCode()
{
    try
    {
        DataTable dt = ReturnSomething();
    }
    catch(Exception e)
    {
        // Print the error message, cleanup, whatever
    }    
}
public DataTable ReturnSomething() throws Exception
{
   try
   {  
      //logic here
     return ds.Tables[0];
   }
   catch (Exception e)
   {
      ErrorString=e.Message;
      throw;
   }
}

PS: Sorry for any syntax error, I'm a bit rusty on C#.

4
  • 2
    You may want to just throw as throw e will alter the exception stack. Mar 2, 2009 at 16:02
  • 2
    Just to note; appending throws onto a method is a Java concept that doesn't exist in .NET.
    – Robula
    Aug 9, 2016 at 12:33
  • AM getting this error ArgumentOutOfRange Exception on a return method : public string GetUserName() { List<LoginModel> list = _connection.Query<LoginModel>("Select username From [LoginModel] where ID=1"); string username = null; try { username = list[0].username; return username; } catch(ArgumentOutOfRangeException e){ } catch (Exception e) { } return username; } Nov 6, 2017 at 8:03
  • Would this compile? There is nothing returned in the CATCH block. Mar 24, 2021 at 18:05
8

You should wrap the caller with a try catch... any exceptions that happen in the routine that is called will bubble out to the caller and you can catch them there.

Personally, I think it is overkill to have a try catch in this routine as you should have the caller handling the exception.

For my example, this would be coded as follows...

private void DoSomething() {
    try {
        DataTable dt = ReturnSomething();
    }
    catch (Exception ex) {
    }    
}

public DataTable ReturnSomething() {
    DataTable dt = new DataTable();

    // logic here
    return dt;
}
6

The ErrorString variable looks suspiciously like an error code variable. Recommended practice is to use exceptions to pass error information directly, where necessary, rather than storing things off into error codes.

You are effectively doing the same thing with your ErrorString as you would be if you just let the exception be caught by the caller: removing the responsibility of responding to an error from the method itself. This is a good goal to have. But the use of an error string doesn't gain you anything over the use of an exception. In fact, you lose information this way. There are any number of types of errors that could occur, and many have special exceptions associated with them, with their own special properties to hold contextual info about the failure. By just storing off the message in a String, you're losing this information.

So unless your goal is specifically to hide the type of error that is occurring from the caller, you can only gain by letting the exception through.

Another thing to consider is whether this is truly an error scenario. If it is, it's very unlikely that your calling method is going to care at all what the return value is. In which case, you have nothing to worry about by just letting the exception go and not returning anything. If it's NOT really an error scenario, and the caller is just going to continue on and do something else, well, that's for the caller to decide, right? There's still not much benefit to obtain by returning an error string and a dummy DataTable or a null, over throwing the exception with all its contextual failure info.

4

If you are going to head the "don't throw an exception route" (which I am not necessarily reccomending), you could follow the TryParse approach MS uses.

Something like:

private string FillDataTable(out DataTable results)
{

  try
{
  results = new DataTable(); //something like this;
  return String.Empty;
}
catch (Exception ex)
{
  results = null;
 return ex.Message;

}

}

3

It depends on you application. You can return null, an empty DataTable or whatever is suitable under circumstances.

1
  • It entirely depends on the circumstances. You should always ask the question "what does it mean if an exception happens here?" rather than follow the same rote practice. Part of the reason of having an exception handler is to, well, HANDLE the exception.
    – plinth
    Mar 2, 2009 at 15:24
3

i'd assume you can still set the message, then return null or whatever the c# equivalent is

public DataTable ReturnSomething(){ 
   try {
        //logic here 
        return ds.Tables[0]; 
   } catch (Exception e) {
        ErrorString=e.Message;
        return null;
   }
}
2

How about this :

public DataTable ReturnSomething(out string errorString)
{
   errorString = string.Empty;
   DataTable dt = new DataTable();
   try
   {  
      //logic here
     dt = ds.Tables[0];
   }
   catch (Exception e)
   {
      errorString = e.Message;
   }
   return dt;
}
3
  • Won't it be a compiler error, since errorString is an out, and it isn't guaranteed to be set? Mar 2, 2009 at 15:24
  • yeap, you're right, I just write it in SO editor :) editing !
    – Canavar
    Mar 2, 2009 at 15:27
  • @ScarletGarden: I do the same thing ;] Mar 2, 2009 at 15:29
1

Since you are cacthing the exception (and not throwing it again) in your example, The outside code assumes everyting is okay and therefor you should return something useful.

If you need to catch the exception there and do somthing that's all fine, but if it's still an error case you should also throw it, or a different exception, perhaps with the one you just caught as InnerException.

0

I think your code is being run at a sufficiently high level of the call stack and it's blended with UI code. If this is really the case, you could return null in the catch block. However, if you are writing reusable code, you should refactor it so that it doesn't contain UI manipulation and handle the exception at a higher level in the call stack.

0

You can do it like the sample code below.

public DataTable ReturnSomething(out string OutputDesc)
{
   try
      {
         //logic here
         OutputDesc = string.Format("Your Successful Message Here...");
         return ds.Tables[0];
      }
      catch (Exception e)
      {
         OutputDesc =e.Message;
         return null;
      }

}
0

Simple

public DataTable ReturnSomething()
{
   try
   {  
      //logic here
     return ds.Tables[0];
   }
   catch (Exception e)
   {
      ErrorString=e.Message;
      throw new Exception(e.Message);        
   }
}

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy