CODEWITHSUNDEEP
c#methodsreturn-value
Joel in Gö
Joel in Gö

Reputation: 7570

How to indicate that a method was unsuccessful

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.

Upvotes: 13

Views: 1605

Answers (16)

Jonathan C Dickinson
Jonathan C Dickinson

Reputation: 7285

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

Upvotes: 0

Samuel Jack
Samuel Jack

Reputation: 33270

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; 
            }
    }
}

Upvotes: 1

artur02
artur02

Reputation: 4479

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

Upvotes: 1

BFree
BFree

Reputation: 103740

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.

Upvotes: 0

ilitirit
ilitirit

Reputation: 16352

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.

Upvotes: 0

Kepboy
Kepboy

Reputation: 3813

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.

Upvotes: 2

Jon Skeet
Jon Skeet

Reputation: 1500385

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.

Upvotes: 7

bltxd
bltxd

Reputation: 9113

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...

Upvotes: 1

Rob
Rob

Reputation: 45771

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

  1. 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
  2. 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.
  3. 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.
  4. 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.

Upvotes: 1

Kevin
Kevin

Reputation: 25937

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).

Upvotes: 5

Aaron Powell
Aaron Powell

Reputation: 25099

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.

Upvotes: 0

Brad Bruce
Brad Bruce

Reputation: 7807

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.

Upvotes: 1

Torbj&#248;rn
Torbj&#248;rn

Reputation: 6503

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).

Upvotes: 0

Jonathan C Dickinson
Jonathan C Dickinson

Reputation: 7285

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!

Upvotes: 0

Luk
Luk

Reputation: 5511

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).

Upvotes: 24

GEOCHET
GEOCHET

Reputation: 21323

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.

Upvotes: 0

Related Questions