2

I have seen similar questions, but not exactly this:

I would like to know the right way of determining whether a method is executed correctly or not, returning a boolean, and if the method is not executed know the reason, even if an exception is thrown.

I do it in this way, but I think that return inside the catch is a bad practice, so which is the right way?:

if(!myObject.DoSomething('A', out result))
{
    MessageBox.Show(myObject.ErrorMessage);
    [...]
}else{
    MessageBox.Show(result);
    [...]
}

class myObject()
{
    public string ErrorMessage;

    bool DoSomething(char inputValue, out string result)
    {
        try
        {
            if(inputValue == 'A')
            {
                ErrorMessage = "Bad input value: " + inputValue;
                return false;
            }
            [...]
            return true;
        }catch(Exception ex){
            ErrorMessage = ex.Message;
            return false;
        }
    }

I don't like trhow the exception inside the catch because I lose the control of the application (and I can't get the description), and the exception always finish in the form. And if I show the exception in the form, I don't need try catch in the rest of the classes.

I mean that try {} catch(Exception ex) { throw ex;} is the same as not putting try catch.

thanks a lot

Kostas Rousis
  • 5,918
  • 1
  • 33
  • 38
user3682831
  • 45
  • 1
  • 1
  • 5
  • 1
    A function that returns a boolean is expecting something to go wrong eventually. Exceptions are `exceptional`. You generally don't want to use both in a same method, so your best bet here is to make your method return `void` (means it's not supposed to go wrong by itself), and handle it inside a try-catch. I'll add that for further information http://codebetter.com/karlseguin/2008/05/30/foundations-of-programming-pt-8-back-to-basics-exceptions/ – Kilazur May 28 '14 at 09:38
  • [See if this helps](http://stackoverflow.com/a/22807771/2530848) – Sriram Sakthivel May 28 '14 at 09:53

3 Answers3

1

My suggestion would be to create your own Exception type (possibly global), and pass it in as a reference.

Thereafter you can still get back your boolean indicating success or failure (and having only one return outside of the try..catch).

 public class CustomException
 {
    private string _message;
    private string _title;

    public CustomException()
    {
      _title = "";
      _message = "";
    }

    public CustomException(string title, string message)
    {
      _title = title;
      _message = message;
    }
 }

Then call DoSomething passing in an instance of CustomException (ce in this case).

CustomException ce = new CustomException();

Be advised this is the best process to solve the problem of having to return a boolean indicating success or failure and know the message, for example; dumping it to a log file or logging to database (particularly for Service Calls - WCF)

However this is not a solution for bad logic in handling business process.

Dane Balia
  • 5,271
  • 5
  • 32
  • 57
  • That's bad coding practice, exceptions should be used to handle exceptional circumstances not business logic – 3dd May 28 '14 at 09:45
  • @3dd They "should", and I agree, yet there are cases when you want to use the power of exceptions in your business logic, see my answer. BUT I may be wrong, and await correction on that matter. – Kilazur May 28 '14 at 09:53
0

Return false inside a catch isn't by itself bad practice. It's useful when you handle a piece of code's exceptions and it must not fail.

For example, I'm working on a printer piloting DLL at the time, and this DLL must read a XML file containing multiple records to print. The method must not fail because one record fails to print, but it still can return exception if the XML file is not correctly formated.

public void Print(string xmlFile)
{
    if (String.IsNullOrWhiteSpace(xmlFile))
        throw new ArgumentNullException("No xml file has been passed to the Print method.");

    // This line will most likely throw an exception if the XMl file is not well formated
    XDocument dom = XDocument.Load(xmlFile);
    foreach (XElement n in dom.XPathSelectElements("//RECORDS/RECORD"))
    {
        try
        {
            // send commands to the printer, if the printer fails to print, throw a PrinterRecordException
        }
        catch (PrinterRecordException e)
        {
            // log print failure, but keep on printing the rest
            continue;
        }
        catch (Exception e)
        {
            // dunno what happened, but still have to print the rest
            continue;
        }
    }
}

In this example, my function could return false instead of throwing exceptions to the main program, if this program doesn't care. In my case it does :p In my opinion, that's how you should think your method.

Kilazur
  • 3,089
  • 1
  • 22
  • 48
  • Ok, but in case of to need abort the function because of business logic, what is the best way to send an error message to the main class? Thanks! – user3682831 May 30 '14 at 09:55
  • The method I presented is typically the kind that has to send messages about errors to the main class, but not all of them. You just have to call it inside a try-catch block. Additionnaly, in the case of a looping method like this one, you could load the XML and loop through it in the main class if you need to get every exceptions. – Kilazur May 30 '14 at 18:24
0

Exception handling methods and best practices are a some-what subjective matter. I cannot attest to the method I'm about to present because I have only just started to use it in my own project.

What I suggest is having a static ExceptionHandler class with which you can register any exception to be handled by Generic Parameter and its corresponding handler. This will decouple your business logic from your UI in case you wanted to display some kind of message box when a particular exception occurs.

Here's an example:

/// the real implementation uses lambda's and/or implementations of IExceptionHandler<TException>
ExceptionHandler.Register<InvalidPasswordException>(() => /*some handler logic*/);

// ... else where in the code ...
catch (InvalidPasswordException ex)
{
    // do resource clean-up and raise exception for listeners such as the UI or logging infrastructure.
    ExceptionHandler.Raise(ex);
}

So far this looks promising, especially when compared with my previous approaches. But only time will tell.

Update

The ExceptionHandler class itself need not be static, for example you might want to have different instances of ExceptionHandlers at different layers of your application if you are using a layered architecture.

rtlayzell
  • 608
  • 4
  • 20