which is better, using a nullable or a boolean return+out parameter

Question

Lets say I have a function that needs to return some integer value. but it can also fail, and I need to know when it does.

Which is the better way?

public int? DoSomethingWonderful()

or

public bool DoSomethingWonderful(out int parameter)

this is probably more of a style question, but I'm still curious which option people would take.

Edit: clarification, this code talks to a black box (lets call it a cloud. no, a black box. no, wait. cloud. yes). I dont care why it failed. I would just need to know if I have a valid value or not.

Answer 1

I would follow the pattern used in some place in the .Net library like:

bool int.TryParse(string s, out value)
bool Dictionary.TryGetValue(T1 key, out T2 value)

So I would say:

public bool TryDoSomethingWonderful(out int parameter)

Answer 2

I like the nullable version better, because you can use the null coalesce operator ?? on it, e.g.:

int reallyTerrible = 0;
var mightBeWonderful = DoSomethingWonderful() ?? reallyTerrible;

Answer 3

If there's only one way it can fail, or if you'll never need to know why it failed, I'd say it's probably simpler and easier to go with the nullable return value.

Conversely, if there are multiple ways it could fail, and the calling code could want to know exactly why it failed, then go with the out parameter and return an error code instead of a bool (alternatively, you could throw an exception, but based on your question, it seems you've already decided not to throw an exception).

Answer 4

I would use the second, because I probably need to know right away if the call succeeded, and in that case I would rather write

int x;
if( DoSomethingWonderful( out x ) )
{
    SomethingElse(x);
}

than

int? x = DoSomethingWonderful();
if( x.HasValue )
{
   SomethingElse(x.Value);
}

Answer 5

It really depends on what you are doing.

Is null a meaningful answer? If not, I would prefer a bool TryDoSomethingWonderful(out int) method call. This matches up with the Framework.

If, however, null is a meaningful return value, returning int? makes sense.

Answer 6

You should rather then use a try catch. This seems like the caller does not know what might happen?

Should we check both bool and the out, or should i check both returns null and the actual return.

Let the method do what it should, and in the case where it failed, let the caller know that it failed, and the caller hanlde as requied.

Answer 7

It depends on how you think the calling code should look like. And therefore what your function is used for.

Generally, you should avoid out arguments. On the other hand, it could be nice to have code like this:

int parameter;
if (DoSomething(out paramameter))
{
  // use parameter
}

When you have a nullable int, it would look like this:

int? result = DoSomething();
if (result != null)
{
  // use result
}

This is somewhat better because you don't have an out argument, but the code that decides if the function succeeded doesn't look very obvious.

Don't forget that there is another option: use Exeptions. Only do this if the case where your function fails is really an exceptional and kind of a error-case.

try
{
  // normal case
  int result = DoSomething()
}
catch (SomethingFailedException ex)
{
  // exceptional case
}

One advantage of the exception is that you can't just ignore it. The normal case is also straight forward to implement. If the exceptional case something you could ignore, you shouldn't use exceptions.

Edit: Forgot to mention: another advantage of an Exception is that you also can provide information why the operation failed. This information is provided by the Exception type, properties of the Exception and the message text.

Answer 8

Why not throw an exception?

Answer 9

Unless performance is the primary concern you should return an int and throw an exception on failure.

Answer 10

I am in favor of using an output parameter. In my opinion, this is the kind of situation for which use of an output parameters is most suited.

Yes, you can use the coalesce operator to keep your code as a one-liner if and only if you have an alternative value that you can use in the rest of your code. I often find that is not the case for me, and I would prefer to execute a different code path than I would if I was successfully able to retrieve a value.

        int value;
        if(DoSomethingWonderful(out value))
        {
            // continue on your merry way
        }
        else
        {
            // oops
            Log("Unable to do something wonderful");

            if (DoSomethingTerrible(out value))
            {
                // continue on your not-so-merry way
            }
            else
            {
                GiveUp();
            }
        }

Additionally, if the value that I want to retrieve is actually nullable, then using a function with an output parameter and a boolean return value is, in my opinion, the easiest way to tell the difference between "I was unsuccessful in retrieving the value" and "The value I retrieved is null". Sometimes I care about that distinction, such as in the following example:

    private int? _Value;
    private bool _ValueCanBeUsed = false;

    public int? Value
    {
        get { return this._Value; }
        set
        {
            this._Value = value;
            this._ValueCanBeUsed = true;
        }
    }

    public bool DoSomethingTerrible(out int? value)
    {
        if (this._ValueCanBeUsed)
        {
            value = this._Value;
            // prevent others from using this value until it has been set again
            this._ValueCanBeUsed = false;
            return true;
        }
        else
        {
            value = null;
            return false;
        }
    }

In my opinion, the only reason most people tend not to use output parameters is because they find the syntax cumbersome. However, I really feel that using output parameters is the more appropriate solution to this problem, and I found that once I got used to it I found the syntax much preferable to returning a null value.

Answer 11

Interestingly enough, my personal opinion sways significantly based on the nature of the method. Specifically, if the method's purpose is to retrieve a single value, as opposing to "doing something".

Ex:

bool GetSerialNumber(out string serialNumber)

vs

string GetSerialNumber() // returns null on failure

The second feels more "natural" to me somehow, and likewise:

bool GetDeviceId(out int id)

vs

int? GetDeviceId() // returns null on failure`

But I admit this really falls into "coding style" territory.

Oh, and I, too, would tend to favor exception throwing:

int GetDeviceId() // throws an exception on read failure

I'm still not sold on why they'd be so wrong. Can we have a thread on that, Oren? ;-)