Best Practice: function return value or byref output parameters?

Question

I have a function called FindSpecificRowValue that takes in a datatable and returns the row number that contains a particular value. If that value isn't found, I want to indicate so to the calling function.

Is the best approach to:

1. Write a function that returns false if not found, true if found, and the found row number as a byref/output parameter, or
2. Write a function that returns an int and pass back -999 if the row value isn't found, the row number if it is?

Answer 1

I would choose option 2. Although I think I would just use -1 not -999.

Richard Harrison is right that a named constant is better than a bare -1 or -999.

Answer 2

functions that fail should throw exceptions.

If failure is part of the expected flow then returning an out of band value is OK, except where you cannot pre-determine what an out-of-band value would be, in which case you have to throw an exception.

If I had to choose between your options I would choose option 2, but use a constant rather than -999...

Answer 3

Personally I would not do either with that method name.

I would instead make two methods:

TryFindSpecificRow
FindSpecificRow

This would follow the pattern of Int32.Parse/TryParse, and in C# they could look like this:

public static Boolean TryFindSpecificRow(DataTable table, out Int32 rowNumber)
{
    if (row-can-be-found)
    {
        rowNumber = index-of-row-that-was-found;
        return true;
    }
    else
    {
        rowNumber = 0; // this value will not be used anyway
        return false;
    }
}

public static Int32 FindSpecificRow(DataTable table)
{
    Int32 rowNumber;


    if (TryFindSpecificRow(table, out rowNumber))
        return rowNumber;
    else
        throw new RowNotFoundException(String.Format("Row {0} was not found", rowNumber));
}

---

Edit: Changed to be more appropriate to the question.

Answer 4

You could also define return value as Nullable and return Nothing if nothing found.

Answer 5

I would go with 2, or some other variation where the return value indicates whether the value was found.

It seems that the value of the row the function returns (or provides a reference to) already indicates whether the value was found. If a value was not found, then it seems to make no sense to provide a row number that doesn't contain the value, so the return value should be -1, or Null, or whatever other value is suitable for the particular language. Otherwise, the fact that a row number was returned indicates the value was found.

Thus, there doesn't seem to be a need for a separate return value to indicate whether the value was found. However, type 1 might be appropriate if it fits with the idioms of the particular language, and the way function calls are performed in it.

Answer 6

Go with 2) but return -1 (or a null reference if returning a reference to the row), that idiom is uses extensively (including by by .nets indexOf (item) functions), it's what I'd probably do.

BTW -1 is more acceptable and widly used "magic number" than -999, thats the only reason why it's "correct" (quotes used there for a reason).

However much of this has to do with what you expect. Should the item always be in there, but you just don't know where? In that case return the index normally, and throw an error/exception if it's not there.

Answer 7

In this case, the item might not be there, and that's an okay condition. It's an error trap for unselected values in a GridView that binds to a datatable.