C# - Exception logging and return status

Question

Modifying to make it clear:

I have a question on exception logging and graceful exit. This is in continuation with previous question. The code looks like:

string status = "0";
ClassA ObjA = new ClassA();

try
{
    status = objA.Method1();
    if (status != "-1")
    {                        
        status = objA.Method1();
    }
 }
 catch (Exception Ex)
 {
     //Log Exception EX
 }

Inside the Method1:

public string Method1()
{
    string status = "0";
    try
    {
        //Code
        return "0";
    }
    catch (Exception Ex)
    {
        //Log Exception with details
        return "-1"
    }
}

I log the Exception in the calling method and return only a status to the caller. Should I return the Exception to the calling method or is only a status sufficient. With a status of "-1", I know there was an Exception in the called method and details of that Exception were logged in a log file.

Should i need to return exception to calling method or only status is sufficient. With -1 status i know exception in the called method and details of exception logged in log file. — Siva, Jan 12 '12 at 09:31
Have you considered Code Review: http://codereview.stackexchange.com/ ? — Michael Banzon, Jan 12 '12 at 09:33

score 2 · Answer 1 · answered Jan 12 '12 at 09:32

I think it is OK to do it like that if you have a lot of status codes, otherwise you could also just throw an exception and catch it in the method higher up.

Also maybe reconsider your return type. Looks like you could be using integers, think you are opening yourself up to errors using strings.

score 1 · Answer 2 · edited May 23 '17 at 12:26

It all depends on the purpose and implementation of the code; sometimes it is better to allow exceptions to pass back to the caller - they should be used in exceptional cases.

If you do intend on using return codes, however, I would be more inclined to use enum's (though, again, it depends what the purpose of the code is). That way, it is easy for the caller to check against an available selection of return codes. Also, a comment on using integers or strings as error codes - it may not be very descriptive for a caller to know what the issue was. In this case, throwing an Exception or a specific type (containing the error message), or returning a pre-defined enum with a descriptive name, would be more meaningful to the caller.

score 1 · Answer 3 · answered Jan 12 '12 at 09:33

From these short code snippets which does nothing it is very difficult to say what is best practice.

In general it is best to push exceptions to where they are handled best. If you are writing a framework for interfacing with some webservice the users of your framework will most likely not care about network exceptions etc. - they want return codes or, even better some framework specific exceptions that you include/code.

Thorsten Dittmar · Answer 4 · 2012-01-12T09:39:10.377

Hm - in your situation I'd rather do the following, but it really depends on the situation:

public string Method1()
{
    string status = "0";

    //Code - Exception may be thrown
    return "0";
}

string status = "0";
ClassA ObjA = new ClassA();
try
{
     status = objA.Method1();
 }
 Catch(Exception Ex)
 {
     //Log Exception EX
    status = "-1;
 }

EDIT
Sometimes it's hard to define values that indicate whether an error occurred in the method. You should keep Nullable types in mind. If you can find a suitable return value that indicates errors, it may also be ok to log the error within the method that caused the error and just react to the return value as you suggested.

By the way: In your code you're calling Method1 twice if the first call succeeded. I guess that is because it is a quick sample...

Thanks for the answer. This is for invoking a service. Its for a Tool, But I would learn to make it more cleaner and better code. — Siva, Jan 12 '12 at 09:36

score 1 · Answer 5 · answered Jan 12 '12 at 09:40

class MyException : Exception
{
   public readonly int status;
   public  MyException(int status, string msg):base(msg)
   {
      this.status = status;
   }
}

public string Method1()
{   
   throw new MyException(-1,"msg");
    return "0";
}




SomeCode()
    {

         try
         {
                  Method1();
         }catch(MyException ex)
         { 
            ex.status //here you get the status
          }
     }

score 1 · Accepted Answer · edited Jan 30 '14 at 09:53

Don't use the status return value, it is not adding anything that is useful to you.

consider,

var a = new ClassA()
try
{
    a.Mehtod1();
}
catch
{
    try
    {
        a.Method1();
    }
    catch (Exception ex)
    {
        //Log without details;
    }
}

class ClassA
{
    void Method1()
    {
        try
        {
             //Code
        }
        catch (Exception ex)
        {
            //Log with details
            throw;
        }            
    }
}

This code achieves the same functionality but leaves the return code of the functions for something useful and non exceptional.

More generally, I suggest that you should have one catch all handler at the top level of your application that deals with logging, or at most one per public entry point. Other handlers should deal with specific exception types that they can actually "handle" (do something about.)

C# - Exception logging and return status

6 Answers6