Which style of return should I use?

Question

This is related to conventions used in C#.

I've got a method that has two parameters (X and Y coordinates). These coordinates represent the position at which a "tile" may reside. If a tile resides at these coordinates, the method returns its number. If no tile resides at these coordinates, I'm wondering how the method should behave.

I see three options:

1. Use exceptions. I may raise an exception every time Method finds no tile. However, as this situation is not rare, this option is the worst one.
2. Do it the old fashioned C++ way and return -1 if there is no tile.
3. Make the tile number a reference parameter and change the return type of method to boolean to show whether there is a tile or not. But this seems a bit complicated to me.

So, what should I do?

Answer 1

You can return null, and check for this on the calling code.

Of course you'd have to use a nullable type:

int? i = YourMethodHere(x, y);

Answer 2

Return -1.

This is not just a C++ convention, it's also common in the .NET Framework - e.g. methods like String.IndexOf or properties like SelectedIndex for controls that represent lists.

EDIT

Just to elaborate, of the three options in your question (Exception, return -1, out parameter), returning -1 is the way to go. Exceptions are for exceptional situations, and the Microsoft coding guidelines recommends avoiding out parameters where possible.

In my view returning -1 (provided it's always going to be an invalid value), returning a nullable int, or returning a Tile object are all acceptable solutions, and you should choose whichever is most consistent with the rest of your app. I can't imagine any developer would have the slightest difficulty with any of the following:

int tileNumber = GetTile(x,y);
if  (tileNumber != -1)
{
   ... use tileNumber ...
}


int? result = GetTile(x,y);
if (result.HasValue)
{
    int tileNumber = result.Value; 
   ... use tileNumber ...
}


Tile tile = GetTile(x,y);
if (tile != null)
{
   ... use tile ...
}

I'm not sure I understand Peter Ruderman's comment about using an int being "much more efficient than returning a nullable type". I'd have thought any difference would be negligible.

Answer 3

If your method has access to the underlying tile objects, another possibility would be to return the tile object itself, or null if there is no such tile.

Answer 4

If the method is part of a low level library, then your standard .NET design probably dictates that you throw exceptions from your method.

This is how the .NET framework generally works. Your higher level callers should catch your exceptions.

However since you seem to be doing this from a UI thread, which has performance implications since you are responding to UI events - I do what Jay Riggs already suggested, return null, and make sure your callers check for a null return value.

Answer 5

Use a nullable return value.

int? GetTile(int x, int y) {
   if (...)
      return SomeValue;
   else
      return null;
}

This is the clearest solution.

Answer 6

Exceptions are for exceptional cases, so using exceptions on a known and expected error situation is "bad". You also are more likely, now, to have try-catches everywhere to handle this error specifically because you expect this error situation to happen.

Making your return value a parameter is acceptable if your only error condition (say -1) is confusable with a real value. If you can have a negative tile number then this is a better way to go.

A nullable int is a possible alternative to a reference parameter but you are creating objects with this so if an "error" is routine they you may be making more work this way than a reference parameter. As Roman pointed out in a comment elsewhere you will have C# vs. VB issues with the nullable type being introduced too late for VB to provide nice syntactic sugar like C# has.

If your tiles can only be non-negative then returning -1 is an acceptable and traditional way to indicate an error. It would also be the least expensive in terms of performance and memory.

---

Something else to consider is self-documentation. Using -1 and an exception are convention: you'd have to write documentation to make sure the developer is aware of them. Using an int? return or a reference parameter would better self-describe itself and wouldn't require documentation for a developer to know how to handle the error situation. Of course :) you should always write the documentation, just like how you should floss your teeth daily.

Answer 7

I would go with option 2. You're right, throwing an exception in such a common case may be bad for performance, and using an out parameter and returning a true or false is useful but screwy to read.

Also, think of the string.IndexOf() method. If nothing is found, it returns -1. I'd follow that example.

Answer 8

You could return -1, as that is a fairly common C# approach. However, it might be better to actually return the tile that was clicked, and in the event that no tile was clicked, return a reference to a singleton NullTile instance. The benefit of doing it this way is that you give a concrete meaning to each value returned, rather than it just being a number that has no intrinsic meaning beyond its numeric value. A type 'NullTile' is very specific as to its meaning, leaving little to doubt for other readers of your code.

Answer 9

I'd break it into two methods. Have something like CheckTileExists(x,y) and GetTile(x,y). The former returns a boolean that indicates whether or not there is a tile at the given coordinates. The second method is essentially the one you're talking about in your original post, except it should throw an exception when given invalid coordinates (since that indicates the caller didn't first call CheckTileExists(), so it is legitimately an exceptional situation. For the sake of speed, you'd probably want these two methods to share a cache, so that in the event they're called one after the other, the overhead on the GetTile() function would be negligible. I don't know if you already have a suitable object to put these methods on or if perhaps you should make them two methods on a new class. IMHO, the performance penalty of this approach is negligible and the increase in code clarity far outweighs it.

Answer 10

Is it possible you have created (or could create) a Tile object that is referenced at the coordinates? If so, you can return a reference to that tile or null if there is no tile at the given coordinates:

public Tile GetTile(int x, int y) {
    if (!TileExists(x, y)) 
        return null;
    // ... tile lookup here...
}

Answer 11

The best options are to return a boolean as well or return null.

e.g.

bool TryGetTile(int x, int y, out int tile);

or,

int? GetTile(int x, int y);

There are several reasons to prefer the "TryGetValue" pattern. For one, it returns a boolean, so client code is incredibly straight forward, eg: if (TryGetValue(out someVal)) { /* some code */ }. Compare this to client code which requires hard-coded sentinel value comparisons (to -1, 0, null, catching a particular set of exceptions, etc.) "Magic numbers" crop up quickly with those designs and factoring out the tight-coupling becomes a chore.

When sentinel values, null, or exceptions are expected it's absolutely vital that you check the documentation on which mechanism is used. If documentation doesn't exist or isn't accessible, a common scenario, then you have to infer based on other evidence, if you make the wrong choice you are simply setting yourself up for a null-reference exception or other bad defects. Whereas, the TryGetValue() pattern is pretty close to self-documenting by it's name and method signature alone.

Answer 12

I have my own opinion on the question that you asked, but it's stated above and I've voted accordingly.

As to the question that you didn't ask, or at least as an extension to all of the answers above: I would be sure to keep the solution to similar situations consistent across the app. In other words, whatever answer you settle on, keep it the same within the app.