Best way to return early from a function returning a reference

Let us say we have a function of the form:

co开发者_Go百科nst SomeObject& SomeScope::ReturnOurObject()
{
    if( ! SomeCondition )
    {
        // return early
        return  ;
    }

    return ourObject;
}

Clearly the code above has an issue, if the condition fails then we have a problem as to how to return from this function. The crux of my question is what is the best way to handle such a situation?

This isn't a syntactic issue, but a design issue. You have to specify what ReturnOurObject() is supposed to return when SomeCondition is true. That depends mainly on what the function is going to be used for. And that you haven't told us.

Depending on the design issues, I see a few possible syntactic ways out of this:

• return a reference to some other object; you would have to have some ersatz object somewhere
• have a special "no-object-to-return" object somewhere that you return a reference to; clients can check for this; if they don't check they get reasonable default behavior
• return a pointer, not a reference; clients would have to always check the return value of the function
• throw an exception; if SomeCondition is something exceptional which clients can not deal with that would be appropriate
• assert; if SomeCondition should always hold, it should be asserted

I'd use a pointer instead of a reference in this case. In fact this criterium (optional or mandatory return value) is how I decide between pointers and references in the first place.

I'd probably do this:

const SomeObject& SomeScope::ReturnOurObject()
{
    if( ! SomeCondition )
    {
        throw SomeException();
    }
    return ourObject;
}

const SomeObject *SomeScope::ReturnOurObjectIfPermitted()
{
    return SomeCondition ? &ourObject : 0;
}

And perhaps also:

bool SomeScope::CheckMode();
    return SomeCondition;
}

Callers then have some options:

// 1 - "I know that I'm in the right mode"
myScope.ReturnOurObject().DoSomething();

// 2 - "I don't know whether I'm in the right mode, but I can cope either way"
if (SomeObject *myObject = myScope.ReturnOurObjectIfPermitted()) {
    myObject->DoSomething();
} else {
    DoSomethingElse();
}

// 2 - alternate version:
if (myScope.CheckMode()) {
    SomeObject &myObject = myScope.ReturnOurObject();
    myObject.DoSomething();
} else {
    DoSomethingElse();
}

// 3 - "I don't know whether I'm in the right mode. If I'm not then
// I can't deal with it now, but some higher-level code can"
try {
    // ... several calls deep ...
    myScope.ReturnOurObject().DoSomething();
    // ... several returns back ...
} catch (SomeException &e) {
    DoSomethingElse();
}

You design a function's interface differently according to what the preconditions are, i.e. whose responsibility it is to ensure it can do its job.

If it's the caller's responsibility, then they need the ability to ensure that. You might want to check anyway, just to be on the safe side. Throwing an exception when the so-called "preconditions" aren't met makes them more like advice than conditions, and it can work quite well provided you don't mind the runtime overhead of checks everywhere. I'm usually fairly unforgiving with preconditions, so actually I might replace the conditional exception with an assert, and document that the function must not be called in the wrong mode. It depends how much control the caller has over the mode - obviously if it changes arbitrarily (e.g., if "SomeCondition" is "there is not a byte available on the UART") then you need a very different interface from if it only ever changes when the client code calls some function to change it.

If it's not the caller's responsibility to get the mode right, then your interface shouldn't write cheques that your implementation can't cash. In this case if it's "expected" that there will be no object to return, then the return should not be by reference. Unless either (a) you can rustle up a special "sorry, it didn't work" object to return, that the caller can check for, or (b) you're comfortable throwing exceptions in "expected" situations. IMO (b) is rare in C++. (a) usually isn't any better than returning a null pointer, but occasionally it is. For instance returning an empty collection to mean "there's nothing you're allowed to see here" might result in some very clean calling code, if DoSomethingElse() would have been "do nothing". So as well as the desired responsibilities, the interface depends on the expected use cases.

If you didn't want to change the return type to a pointer, you could possibly use the null object pattern

What's the lifetime of 'ourObject' and where is it created?

If you need to return early without touching/updating ourObject and it exists at the point where you're returning, I don't see a problem with returning a reference to it.

If you're trying to communicate that there has been an error, you'll probably have to throw an exception instead of returning early because the contract your function agreed to says that it will return a reference to 'SomeObject'.

If you're returning a reference to a temporary, you better fix that issue...

I would return a boolean and pass the object reference as parameter to the function:

bool getObject(SomeObject& object)
{
    if( condition )
         return false;

    object = ourObject;
    return true;
}

In that case you now your object was filled only if the function return true.

Uh... just throw an exception...

Throw an exception

I'll consider returning by reference only if I am sure that the function will never fail (Probably like returning a const-reference to an internal member variable). If there are multiple validations inside the function then I will consider one of the following options:

• If the copying the object is not costly, return a copy
• If it is costly to copy, return a const-pointer. Return NULL in case of failure.

If you don't want to throw an exception, you could try something like:

const SomeObject& GetSomeObject(const SomeObject& default_if_not_found = SomeObject())
{
    // ...
    if (!found)
        return default_if_not_found;

    return ourObject;
}

This means you don't have to have some kind of static variable lying around somewhere to return, and you can still call it with no parameters. I guess you'd assume the default-constructed SomeObject() state is the empty state so you can test SomeObject to see if it is in the empty state. It's also sometimes useful to use this to override the default return value, eg. GetSomeObject(SomeObject("I'm empty"))

Exactly three choices...

1.   Throw an exception
2.   Return a dummy error object
3.   Use a pointer and return NULL

I believe the answer depends on whether you are the kind of programmer who considers exceptions as bad and never wants to use them or not. If you do use them, AND this really is an exceptional condition, then an exception must be thrown. Yes, users need to deal with it, but hiding an error to pretend that nothing has happened is simply not an option. But if you never use exceptions, then just return a pointer, or a boolean, and add a non-const reference to SomeObject to your function. Both solutions are simple, but who said that over-complicating things makes code better?

The way the function is set up, you have to return a SomeObject or throw an exception. If you want to do something else, you need to change the function. There are no other choices.

If SomeCondition will never be false, then you can remove the test in the code and put an assert in instead, or you can keep the test and throw an exception. (I'm loath to recommend just disregarding the possibility, since "never false" almost always means "never false unless something bad has happened and not been detected", and if it's worth testing for under any conditions it's worth detecting.)

There's the possibility of returning some sort of null object. Pointers are useful for that, since there's an obvious null value that tests false, but you could pass a flag back as an out parameter or return a special object intended as a null. Then you're going to need to test the return value on each call for this to be meaningful, and that's not likely to be less hassle than the assert or throw.

If I were doing this, it would depend on SomeCondition. If it has side effects or isn't difficult to calculate, test and throw an exception if false. If it's onerous to calculate, use assert so you don't have to do it in production, and forget about it besides that.

This is the same situation as the built-in C++ operator dynamic_cast<>. It can cast to either a pointer type or a reference type. When it is a pointer type, it returns NULL on failure. When it is a reference type, it throws the exception std::bad_cast on failure. Following the same example, you would throw an exception.