CODEWITHSUNDEEP
c#.netexceptioncoding-styletry-catch
WisperWordsOfwisdom_code_in_c_
WisperWordsOfwisdom_code_in_c_

Reputation: 73

try & catch structure in C#

I'm new to programming and was wanting to ask, is the code shown below a good way to use the try catch within a boolean method?

It's just example code but I have many methods within my Presenter classes and was wondering the way I placed the catch just returning false, is this ok to do, or how else could I improve this

public bool TestMethod()
{
    try
    {
       if(true)
       { 
         //some random code
         return true;
       }
       else{return false;}
    }
    catch{return false;}
}

I just wanted to be sure it a good way to implement this, I would appreciate any feedback on how this could improved.

Upvotes: 3

Views: 1683

Answers (7)

Xaqron
Xaqron

Reputation: 30887

The point here is readability and everybody has his style (this is not a matter of right and wrong). What I do is declaring a return value (as some guys did here like retVal, returnValue,...), my favorite is result which in your case should be a bool.

Then playing with this result variable in method body i.e inside try/catch and DO NOT return anything unless at the end of method where I return result. Personally use return where the remaining code (after return) should not be run.

This way the reader would not confused of so many returns. You may think about performance benefits of early returns which I doubt (if compiler optimization cannot do it for you) it has a little effect in the world of frameworks and layers, where we are sacrificing performance for design issues in a daily basis.

Finally, take the point CodeInChaos said seriously. If you don't know an exception reason and would not take care of it in a graceful fashion, leave it to be thrown. Exceptions are not bad things, they are like pain in medicine. A chronic diabetic person who has lost the pain in his feet, will not notice small injuries in his feet and a google search will should you the pictures resulting of ignoring painful signals.

Upvotes: 1

coder_bro
coder_bro

Reputation: 10773

Here are some points, which I find a bit discerning about the code in the question:

There are multiple return statements in the code at various places, this may be confusing to the reader of the code. We generally tend to follow a single return statement in a function. (All though there are some exceptions to rule, like an early return in case of some error condition)

Generally you should never hide an exception from the user (or some say "never swallow an exception"), you should either rethrow it or handle the exception and display it to the user.

In the least, there should be some log of the exception.

So with these points in mind, the above code can be written as:

public bool TestMethod()
{
    bool returnValue = false;
    try
    {
       if(true)
       { 
         //some random code
         returnValue = true;
       }       
    }
    catch(Exception ex)
    {
         // log the exception here, or rethrow it
    }

    return returnValue;
}

Upvotes: 3

CodesInChaos
CodesInChaos

Reputation: 108880

Don't use a catch-all in such a way. Catching all exceptions is acceptable for the top level exception handler. But it shouldn't just swallow them. But log them and perhaps display an error.

For your code you should only catch the specific exception types you're expecting. And I'm not sure if in your example exceptions are a good idea at all.

Upvotes: 4

Alexander Schmidt
Alexander Schmidt

Reputation: 5743

IMHO its not the purpose of the catch-block to return values. I'll use it this way:


public bool TestMethod()
{
    bool retVal = false;
    try
    {
        if (true)
        {
            //some random code
            retVal = true;
        }
        else{}
    }
    catch{}
    return retVal;
}

Upvotes: 2

Pabuc
Pabuc

Reputation: 5638

It depends on what you do in your if block. If you return false on an unexpected error, you'll never be able to know what went wrong. If your code is simple enough and you know for what went wrong, it is a good usage.

For example, it is a good approach for a method which checks if the given string is a valid number and multiplier of 5. If it is not a number, you'll get an exception and you don't need to log it or nothing else can cause an exception.

Upvotes: 1

Rudrik
Rudrik

Reputation: 832

http://msdn.microsoft.com/en-us/library/0yd65esw%28v=vs.80%29.aspx

Upvotes: 1

Timwi
Timwi

Reputation: 66614

It is certainly OK to return directly from inside a catch clause, if that’s what you’re asking. Otherwise the compiler would prohibit it; for example, you can’t return from inside a finally clause.

In your particular code example, however, the catch clause seems pointless, because the code inside the try is not going to throw any exceptions (except for system-level exceptions like OutOfMemoryException, but that’s... exceptional, anyway). It would make more sense if there were a throw statement inside it, or a call to a method that you think might throw (for example, File.Open could throw a FileNotFoundException).

Regarding code style, I would recommend that you use indentation a bit more:

public bool TestMethod()
{
    try
    {
        if (some_condition)
        { 
            //some random code
            return true;
        }
        else
        {
            return false;
        }
    }
    catch
    {
        return false;
    }
}

Upvotes: 0

Related Questions