Try - Catch return strategy

When using try-catch in methods, if you want you application to continue even if errors come along, is it okay to return the default value as return through the catch block, and log the error for later review?

For Example:

public static string GetNameById(int id)
{
    string name;
    try
    {
        //Connect to Db and get name - some error occured
    }
    catch(Exception ex)
    {
        Log(ex);
        name = String.Empty;
    }

    return name;
}

Example 2:

public static string GetIdByName(string name)
{
    int id;
    try
    {
        //Connect to Db and get id- some error occured
    }
    catch(Exception ex)
    {
        Log(ex);
        id = 0;
    }

    return id;
}

Is it okay to return any default value (depending on the return type of the method ...???) so that the application logic that required the result from this method do not crash and keeps going ....

Thanks in advance...

Regar开发者_运维百科ds.

The advice for exception handling is that mostly you should only catch exceptions that you can do something about (e.g. retry an operation, try a different approach, elevate security etc). If you have logging elsewhere in your application at a global level, this kind of catch-log-return is unnecessary.

Personally - typically - in this situation I'd do this:

public static string GetNameById(int id)
{
    string name;
    try
    {
        //Connect to Db and get name - some error occured
    }
    catch(Exception ex)
    {
        Log(ex);
        throw; // Re-throw the exception, don't throw a new one...
    }

    return name;
}

So as usual - it depends.

Be aware of other pitfalls though, such as the calling method not being aware that there was a failure, and continuing to perform work on the assumption that the method throwing the exception actually worked. At this point you start the conversation about "return codes vs. throwing exceptions", which you'll find a lot of resources for both on SO.com and the internets in general.

I do not think that is a good solution. In my opinion it would be better to let the caller handle the exception. Alternatively you can catch the exception in the method and throw a custom exception (with the caught exception as the inner exception).

Another way of going about it would be to make a TryGet method, such as:

public static bool TryGetNameById(int id, out string name) 
{ 
    try 
    { 
         //Connect to Db and get name - some error occured
         name = actualName
         return true; 
    } 
    catch(Exception ex) 
    { 
        Log(ex); 
        name = String.Empty; 
        return false;
    }  
} 

I think this approach is more intention revealing. The method name itself communicates that it may not always be able to produce a useful result. In my opinion this is better than returning some default value that the caller has to be able to know about to do the correct business logic.

My opinion is I'll never mute errors.

If some exception is thrown, it should be treated as a fatal error.

Maybe the problem is throwing exceptions for things that aren't exceptions. For example, business validation shoudn't be throwing such exceptions.

I'd prefer to validate the business and translate the errors in "broken rules" and transmit them to the presentation or wherever, that would save CPU and memory because there's no stack trace.

Another situation is a data connection loss or another situation that makes the application fall in a wrong state. Then, I'd prefer to log the error and prompt the user to re-open the application or maybe the application may restart itself.

I want to make you some suggestion: have you ever heard about PostSharp? Check it, you can implement that exception logging with an AOP paradigm.

It is advised that you only catch errors that you can handle and recover from. Catching and consuming exceptions that you cannot handle is bad form. However, some environments / applications define strict rules that go against this ideal behaviour.

In those cases, I would say in cases you don't have a choice, you can do what you like with returns - it is up to your application to work with them.

Based on the assumption that your application can handle any sort of failure in trying to get an ID, then returning a default ID is a good enough choice. You can go further and use something like the special case pattern to return empty / missing objects, but for simple data types this seems unwarranted.

this depends very much on the context and on the way your calling method is designed. I have used to return default values in the past as you are doing now and I understood only afterwards that it was deeply wrong...

you should imagine the catch block to throw the exception back, after logging it properly, then the calling method could have another try catch and depending on the severity of the error could inform the user or behave accordingly.

the fact is that if you return an empty string, in some cases, could be that the caller "thinks" there is a user with empty name, while would probably be better to notify the user that the record was not found, for example.

depending on the way your logger works you could log the exception only where you handle it and not in all catches...

That depends on what you want. If it's important for you that you log the exception, but that everything else keeps working, then this is - in my honest opinion- ok.

On the other hand: if an exception occurs, you have to make sure this does not have an impact on the further working of your application.

The point of a try/catch is to allow you to catch and handle errors in a graceful manor, allowing application execution to continue, rather than simply crashing the application and stopping execution.

Therefore it is perfectly acceptable to return a default value. However, be sure that all following code will continue to function if a default value is returned rather than causing further errors.

Eg - in your example, if the DB connection fails, ensure there are no further commands to edit / retrieve values from the database.

It REALLY depends. In most scenarios it is not - problems i that if there is a scenario specific issue you amy never find out. Soemtiems a recovery attempt is good. This normalyl depends on circumstances and actual logic.

The scenarios where "swallow and document" are really valid are rare and far in between in the normal world. They come in more often when writing driver type of thigns (like loaded moduels talkign to an external system). Whether a return value or default(T) equivalent maeks sense also depends.

I would say in 99% of the cases let the exception run up. In 1% of the cases it may well make sense to return some sensible default.

It is better to log the exception but then re-trow it so that it can be properly handled in the upper layers.

Note: the "log the exception part" is of course optional and depends a lot on your handling strategy(will you log it again somewhere else?)

Instead of making the app not crash by swalowing exception, it is better to let them pass and try to find the root cause why they were thrown in the first place.

Depends on what you want your app to do... For example, display a friendly message, "Cannot Login" if you get a SqlException when trying to connect to the Database is OK.

Handling all errors is sometimes considered bad, although people will disagree... Your application encountered an exception you did not expect, you can no longer be 100% sure what state your application is in, which lines of codes executed and which didn't, etc. Further reading on this: http://blogs.msdn.com/b/larryosterman/archive/2005/05/31/423507.aspx. And more here : http://blogs.msdn.com/b/oldnewthing/archive/2011/01/20/10117963.aspx

I think the answer would have to be "it depends".

In some cases it may be acceptable to return an empty string from a function in case of an error. If you are looking up somebody's address to display then an empty string works fine rather than crashing the whole thing.

In other cases it may not work so well. If you are returning a string to use for a security lookup for example (eg getSecurityGroup) then you are going to have very undesired behaviour if you return the wrong thing and you might be better off keeping the error thrown and letting the user know something has gone wrong rather than pretending otherwise.

Even worse might be if you are persisting data provided to you by the user. Something goes wrong when you are getting their data, you return a default and store that without even telling them... That's gotta be bad.

So I think you need to look at each method where you are considering this and decide if it makes sense to throw an error or return a default value.

In fact as a more general rule any time you catch an error you should be thinking hard about whether it is an acceptable error and continuing is permissable or whether it is a show stopping error and you should just give up.