c# - How to indicate that a method was unsuccessful

Question

I have several similar methods, say eg. CalculatePoint(...) and CalculateListOfPoints(...). Occasionally, they may not succeed, and need to indicate this to the caller. For CalculateListOfPoints, which returns a generic List, I could return an empty list and require the caller to check this; however Point is a value type and so I can't return null there.

Ideally I would like the methods to 'look' similar; one solution could be to define them as

public Point CalculatePoint(... out Boolean boSuccess);
public List<Point> CalculateListOfPoints(... out Boolean boSuccess);

or alternatively to return a Point? for CalculatePoint, and return null to indicate failure. That would mean having to cast back to the non-nullable type though, which seems excessive.

Another route would be to return the Boolean boSuccess, have the result (Point or List) as an 'out' parameter, and call them TryToCalculatePoint or something...

What is best practice?

Edit: I do not want to use Exceptions for flow control! Failure is sometimes expected.

score 24 · Accepted Answer

Personally, I think I'd use the same idea as TryParse() : using an out parameter to output the real value, and returning a boolean indicating whether the call was successful or not

public bool CalculatePoint(... out Point result);

I am not a fan of using exception for "normal" behaviors (if you expect the function not to work for some entries).

score 7 · Accepted Answer

Why would they fail? If it's because of something the caller has done (i.e. the arguments provided) then throwing ArgumentException is entirely appropriate. A Try[...] method which avoids the exception is fine.

I think it's a good idea to provide the version which throws an exception though, so that callers who expect that they will always provide good data will receive a suitably strong message (i.e. an exception) if they're ever wrong.

score 5 · Accepted Answer

Another alternative is to throw an exception. However, you generally only want to throw exceptions in "exceptional cases".

If the failure cases are common (and not exceptional), then you've already listed out your two options. EDIT: There may be a convention in your project as how to handle such non-exceptional cases (whether one should return success or the object). If there is no existing convention, then I agree with lucasbfr and suggest you return success (which agrees with how TryParse(...) is designed).

score 2 · Accepted Answer

If the failure is for a specific reason then I think its ok to return null, or bool and have an out parameter. If however you return null regardless of the failure then I don't recommend it. Exceptions provide a rich set of information including the reason WHY something failed, if all you get back is a null then how do you know if its because the data is wrong, you've ran out of memory or some other weird behavior.

Even in .net the TryParse has a Parse brother so that you can get the exception if you want to.

If I provided a TrySomething method I would also provide a Something method that threw an exception in the event of failure. Then it's up to the caller.

score 1 · Accepted Answer

The model I've used is the same one MS uses with the TryParse methods of various classes.

Your original code:
public Point CalculatePoint(... out Boolean boSuccess);
public List CalculateListOfPoints(... out Boolean boSuccess);

Would turn into public bool CalculatePoint(... out (or ref) Point CalculatedValue);
public bool CalculateListOfPoints(... out (or ref) List CalculatedValues);

Basically you make the success/failure the return value.

score 1 · Accepted Answer

To summarise there are a couple of approaches you can take:

When the return type is a value-type, like Point, use the Nullable feature of C# and return a Point? (aka Nullable), that way you can still return null on a failure
Throw an exception when there's a failure. The whole argument/discussion regarding what is and isn't "exceptional" is a moot point, it's your API, you decide what's exceptional behaviour.
Adopt a model similar to that implemented by Microsoft in the base types like Int32, provide a CalculatePoint and TryCalculatePoint (int32.Parse and int32.TryParse) and have one throw and one return a bool.
Return a generic struct from your methods that has two properties, bool Success and GenericType Value.

Dependent on the scenario I tend to use a combination of returning null or throwing an exception as they seem "cleanest" to me and fit best with the existing codebase at the company I work for. So my personal best practice would be approaches 1 and 2.

score 1 · Accepted Answer

It mostly depends on the behavior of your methods and their usage.

If failure is common and non-critical, then have your methods return a boolean indicating their success and use an out parameter to convey the result. Looking up a key in a hash, attempting to read data on a non-blocking socket when no data is available, all these examples fall in that category.

If failure is unexpected, return directly the result and convey errors with exceptions. Opening a file read-only, connecting to a TCP server, are good candidates.

And sometimes both ways make sense...

score 1 · Accepted Answer

Return Point.Empty. It's a .NET design patter to return a special field when you want to check if structure creation was successful. Avoid out parameters when you can.

public static readonly Point Empty

score 1 · Accepted Answer

A pattern that I'm experimenting with is returning a Maybe. It has the semantics of the TryParse pattern, but a similar signature to the null-return-on-error pattern.

I'm not yet convinced one way or the other, but I offer it for your collective consideration. It does have the benefit of not requiring a variable to defined before the method call to hold the out parameter at the call site of the method. It could also be extended with an Errors or Messages collection to indicate the reason for the failure.

The Maybe class looks something like this:

/// <summary>
/// Represents the return value from an operation that might fail
/// </summary>
/// <typeparam name="T"></typeparam>
public struct Maybe<T>
{
    T _value;
    bool _hasValue;


    public Maybe(T value)
    {
        _value = value;
        _hasValue = true;
    }

    public Maybe()
    {
        _hasValue = false;
        _value = default(T);
    }


    public bool Success
    {
        get { return _hasValue; }
    }


    public T Value
    {
        get 
            { // could throw an exception if _hasValue is false
              return _value; 
            }
    }
}

score 0 · Accepted Answer

I would say best practice is a return value means success, and an exception means failure.

I see no reason in the examples you provided that you shouldn't be using exceptions in the event of a failure.

score 0 · Accepted Answer

Using an exception is a bad idea in some cases (especially when writing a server). You would need two flavors of the method. Also look at the dictionary class for an indication of what you should do.

// NB:  A bool is the return value. 
//      This makes it possible to put this beast in if statements.
public bool TryCalculatePoint(... out Point result) { }

public Point CalculatePoint(...)
{
   Point result;
   if(!TryCalculatePoint(... out result))
       throw new BogusPointException();
   return result;
}

Best of both worlds!

score 0 · Accepted Answer

The bool TrySomething() is at least a practice, which works ok for .net's parse methods, but I don't think I like it in general.

Throwing an exception is often a good thing, though it should not be used for situations you would expect to happen in many normal situations, and it has an associated performance cost.

Returning null when possible is in most cases ok, when you don't want an exception.

However - your approach is a bit procedural - what about creating something like a PointCalculator class - taking the required data as parameters in the constructor? Then you call CalculatePoint on it, and access the result through properties (separate properties for Point and for Success).

score 0 · Accepted Answer

You don't want to be throwing exceptions when there is something expected happening, as @Kevin stated exceptions are for exceptional cases.

You should return something that is expected for the 'failure', generally null is my choice of bad return.

The documentation for your method should inform the users of what to expect when the data does not compute.

score 0 · Accepted Answer

We once wrote an entire Framework where all the public methods either returned true (executed successfully) or false (an error occurred). If we needed to return a value we used output parameters. Contrary to popular belief, this way of programming actually simplified a lot of our code.

score 0 · Accepted Answer

Well with Point, you can send back Point.Empty as a return value in case of failure. Now all this really does is return a point with 0 for the X and Y value, so if that can be a valid return value, I'd stay away from that, but if your method will never return a (0,0) point, then you can use that.

score 0 · Accepted Answer

Sorry, I just remembered the Nullable type, you should look at that. I am not too sure what the overhead is though.

c# - How to indicate that a method was unsuccessful

16 回答 16

Related

Reference