0
\$\begingroup\$

In Framework Design guideline book there is a chapter about Exception and they talk about return-value-based error reporting and exception based error reporting and the fact that we in a O.O language like C# we should avoid return-value-based error reporting and use exceptions. With that in mind I was looking at our code that eight years ago was written in Visual Basic and last year with a automatic tool got converted to C#!

So here is a method I am looking at, I was wondering if the advice from that book applies to such a method and if yes, then what would be a better approach for rewriting this method?

public int Update(CaseStep oCaseStepIn)
{
    int result = 0;
    //Update the master object with the passed in object

    result = UCommonIndep.gnUPDATE_FAILED;
    if (Validate(oCaseStepIn) == UCommonIndep.gnVALIDATE_FAILED)
    {
        return result;
    }

    CaseStep oCaseStep = get_ItemByObjectKey(oCaseStepIn.CopyOfObjectKey);
    if (oCaseStep == null)
    {
        return result;
    }

    if (oCaseStep.Update(oCaseStepIn) == UCommonIndep.gnUPDATE_SUCCESSFUL)
    {
        //*******************************
        //FYI - Insert code here to update any Key values that might have changed.
        result = UCommonIndep.gnUPDATE_SUCCESSFUL;
    }
    return result;
}
\$\endgroup\$
3
\$\begingroup\$

Yes, the refactoring "Replace error codes with exceptions" work perfectly here. As a first step you need to create an exception type that suits your need, something like

public class UpdateException : Exception
{
    //your custom properties, fields etc.
}

and then use it in your method (an example below)

public void Update(CaseStep oCaseStepIn)
{
    //Update the master object with the passed in object

    if (Validate(oCaseStepIn) == UCommonIndep.gnVALIDATE_FAILED) 
        throw new UpdateException(); //pass in constructor useful messages

    CaseStep oCaseStep = get_ItemByObjectKey(oCaseStepIn.CopyOfObjectKey);
    if (oCaseStep == null)
        throw new UpdateException(); //pass in constructor useful messages


    if (oCaseStep.Update(oCaseStepIn) != UCommonIndep.gnUPDATE_SUCCESSFUL)
        throw new UpdateException(); //pass in constructor useful messages

    //*******************************
    //FYI - Insert code here to update any Key values that might have changed.
}

You can create different Exception subclasses for each type of error. It's even better.

\$\endgroup\$
  • 1
    \$\begingroup\$ The code would look even better if you applied the same refactoring to the methods you're calling. I'm not sure it should be applied for all of them (e.g. it makes sense that Validate() returns some result and doesn't throw), but it certainly makes sense at least in some cases (like oCaseStep.Update()). \$\endgroup\$ – svick Apr 20 '12 at 15:09
  • \$\begingroup\$ This looks like a hybrid (mix of return code enums and exceptions) to me. Obviously you do not have full access to the original code and cannot read the poster's mind, but this particular snippet is not so much better, that it is obvious that one should lean toward using exceptions. \$\endgroup\$ – Leonid Apr 20 '12 at 15:34
  • \$\begingroup\$ @svick Yes, you are right, this is a great recommendation; the example is only on what we have in the original post :) (+1) \$\endgroup\$ – cgilmeanu Apr 20 '12 at 16:26

Your Answer

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

Not the answer you're looking for? Browse other questions tagged or ask your own question.