13

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.

Joel in Gö
  • 7,460
  • 9
  • 47
  • 77
  • Just a design point... If you're returning a collection, if you're not successful don't return a null, just return an empty collection. I vote for TryCalculate returning a boolean. –  Oct 02 '08 at 11:39
  • A method not completing due to a out of range parameter is not 'flow control' it is an unexpected and exceptional condition. – GEOCHET Oct 02 '08 at 11:41
  • @Will: I agree, that's what I meant by an empty list. @Rich: Agreed; but the parameters can be fine, there just may not be an appropriate Point to return. – Joel in Gö Oct 02 '08 at 11:44
  • @Joel: Then you may want the try pattern, however, for what you listed in the question, you should throw an exception. – GEOCHET Oct 02 '08 at 11:47

16 Answers16

24

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

Luk
  • 5,371
  • 4
  • 40
  • 55
  • @lucasbfr: +1, if you provide DoX which could throw an exception you can figure out, provide TryX. – user7116 Oct 02 '08 at 11:38
  • @sixlettervariables: I agree, I didn't want to change the method's name because he didn't ask but that would be a very good practice – Luk Oct 02 '08 at 11:45
  • While the TryParse concept sometimes gets tedious because you always need to declare a variable, I got used to it and always prefer it over an exception-throwing version. Whenever you can just return null, however, I do that instead of the TryParse approach. – OregonGhost Oct 02 '08 at 11:45
  • If you are going to do this I'd change the method name to Try calculate point. – Omar Kooheji Oct 02 '08 at 12:38
7

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.

Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194
5

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

Kevin
  • 25,207
  • 17
  • 54
  • 57
2

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.

Kepboy
  • 3,733
  • 2
  • 30
  • 43
1

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.

Brad Bruce
  • 7,638
  • 3
  • 39
  • 60
1

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.

Rob
  • 45,296
  • 24
  • 122
  • 150
1

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

bltxd
  • 8,825
  • 5
  • 30
  • 44
1

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
artur02
  • 4,469
  • 1
  • 22
  • 15
  • Why avoid out parameters? I do, in fact try to avoid them, but that's because I find them less clear in intent. – Joel in Gö Oct 02 '08 at 22:10
  • Out parameters (and other kind of pointers) can be tricky for beginners. It's a typical source of semantic errors. Microsoft also suggests to avoid it: http://msdn.microsoft.com/en-us/library/ms182131(VS.80).aspx – artur02 Oct 04 '08 at 08:11
1

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; 
            }
    }
}
Samuel Jack
  • 32,712
  • 16
  • 118
  • 155
0

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.

GEOCHET
  • 21,119
  • 15
  • 74
  • 98
  • Exceptions should not be used in non-exceptional cases. You need to be careful here. – Kevin Oct 02 '08 at 11:37
  • I am not a fan of using exception for "normal" behaviors (if you expect the function not to work for some entries). – Luk Oct 02 '08 at 11:38
  • @Kevin: An exceptional case like not being able to complete a method? – GEOCHET Oct 02 '08 at 11:39
  • @lucasbfr: Please explain where you see the failure of this method as a 'normal' behavior. I do not see this verbage at all in the question. – GEOCHET Oct 02 '08 at 11:40
  • What about games, servers and graphical applications? His example is indicative of at least a game or a graphical application. Exceptions are slow: very, very slow. Always give potential developers a way to avoid exceptions. – Jonathan C Dickinson Oct 02 '08 at 11:43
  • Rich B - if this kind of failure is expected then I wouldn't count it as an exceptional case – Aaron Powell Oct 02 '08 at 11:44
  • @Kevin: TryParse is a completely different subject than exceptions, and purposefully so. If you have a method, and it accepts input, and that input leads to a condition where the method can not return in the expected way, an exception should be thrown. End of story. – GEOCHET Oct 02 '08 at 11:45
  • @Jonathon: That is ridiculous. An exception wouldn't be used as flow control. There is no reason he shouldn't be bound checking his input in the first place. But to not use exceptions because of being in a game or server? That is ridiculous. – GEOCHET Oct 02 '08 at 11:46
  • See edit: this is certainly not an appropriate situation for an exception. – Joel in Gö Oct 02 '08 at 11:46
  • And the failure has nothing to do with bounds or dirty input; it is indeed a graphical app and the method fails if there is no point which satisfies a set of complicated conditions – Joel in Gö Oct 02 '08 at 11:49
  • @Joel: Then a try method might be your best solution, however, I think (as another person answered) you might want to refactor this part of your code. I think you are approaching this wrong. – GEOCHET Oct 02 '08 at 11:51
  • @Rich: Why is the approach wrong? (this is a genuine question!) How would you suggest I refactor? – Joel in Gö Oct 02 '08 at 11:52
  • @Rich B: Read these links: http://stackoverflow.com/questions/77127/when-to-throw-an-exception http://www.research.att.com/~bs/bs_faq2.html#exceptions – Kevin Oct 02 '08 at 12:08
  • @Joel and Kevin: I know where to use exceptions. If your code was structured properly this would not be an issue. Anytime you are at a situation where you think returning an error code might be beneficial, there is something wrong with your code. – GEOCHET Oct 02 '08 at 13:30
0

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!

Jonathan C Dickinson
  • 7,181
  • 4
  • 35
  • 46
  • 1
    I agree the try idea is a good one in some cases, but your statement that throwing an exception in a server is so wrong I am not sure where to start. – GEOCHET Oct 02 '08 at 11:42
  • I didn't say never throw exceptions :). Only when you absolutely need to. How would you handle XML parsing errors on a XMPP server with, say, 10000 active users? Writing a TCP server for a vast userbase is vastly different from web apps where connections are only held for a few millis. – Jonathan C Dickinson Oct 02 '08 at 12:46
  • @Jonathon: Saying that for a server you shouldn't throw an exception is just plain wrong. It indicates to me that you are using exceptions for flow control which is even more wrong. – GEOCHET Oct 02 '08 at 13:33
  • @rich-b not using it for flow control. Got a patricia trie in my server, used for every stanza that my server gets (5000 p/m). If the node can't be found it is sent to another xmpp server via S2S. Should I throw exceptions here? Or return false? – Jonathan C Dickinson Oct 08 '08 at 11:00
0

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

Torbjørn
  • 6,423
  • 5
  • 29
  • 42
0

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.

Aaron Powell
  • 24,927
  • 18
  • 98
  • 150
0

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.

ilitirit
  • 16,016
  • 18
  • 72
  • 111
  • Oh god. If you did this in a language with exception handling, please post the code. I will have a new front page story for TDWTF. – GEOCHET Oct 02 '08 at 13:33
0

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.

BFree
  • 102,548
  • 21
  • 159
  • 201
0

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

Jonathan C Dickinson
  • 7,181
  • 4
  • 35
  • 46