Is there a name for this anti-pattern/code smell?

Question

Let me start by saying that I do not advocate this approach, but I saw it recently and I was wondering if there was a name for it I could use to point the guilty party to. So here goes.

Now you have a method, and you want to return a value. You also want to return an error code. Of course, exceptions are a much better choice, but for whatever reason you want an error code instead. Remember, I'm playing devil's advocate here. So you create a generic class, like this:

class FunctionResult<T>
{
    public T payload;
    public int result;
}

And then declare your functions like this:

FunctionResult<string> MyFunction()
{
    FunctionResult<string> result;
    //...

    return result;
}

One variation on this pattern is to use an enum for the error code instead of a string. Now, back to my question: is there a name for this, and if so what is it?

Answer 1

How about the "Can't decide whether this is an error or not" pattern. Seems like if you really had an exception but wanted to return a partial result, you'd wrap the result in the exception.

Answer 2

I am not sure this is an anti-pattern. I have commonly seen this used instead of exceptions for performance reasons, or perhaps to make the fact that the method can fail more explicit. To me, it seems to be a personal preference rather than an anti-pattern.

Answer 3

Replace Error Code with Exception

Answer 4

Well, it's not an antipattern. The C++ standard library makes use of this feature and .NET even offers a special FunctionResult class in the .NET framework. It's called Nullable. Yes, this isn't restricted to function results but it can be used for such cases and is actually very useful here. If .NET 1.0 had already had the Nullable class, it would certainly have been used for the NumberType.TryParse methods, instead of the out parameter.

Answer 5

This approach is actually much better than some others that I have seen. Some functions in C, for example, when they encounter an error they return and seem to succeed. The only way to tell that they failed is to call a function that will get the latest error.

I spent hours trying to debug semaphore code on my MacBook before I finally found out that sem_init doesn't work on OSX! It compiled without error and ran without causing any errors - yet the semaphore didn't work and I couldn't figure out why. I pity the people that port an application that uses POSIX semaphores to OSX and must deal with resource contention issues that have already been debugged.

Answer 6

I usually pass the payload as (not const) reference and the error code as a return value.

I'm a game developer, we banish exceptions

Answer 7

I'd agree that this isn't specifically an antipattern. It might be a smell depending upon the usage. There are reasons why one would actually not want to use exceptions (e.g. the errors being returned are not 'exceptional', for starters).

There are instances where you want to have a service return a common model for its results, including both errors and good values. This might be wrapped by a low level service interaction that translates the result into an exception or other error structure, but at the level of the service, it lets the service return a result and a status code without having to define some exception structure that might have to be translated across a remote boundary.

This code may not necessarily be an error either: consider an HTTP response, which consists of a lot of different data, including a status code, along with the body of the response.

Answer 8

If you don't want to use exceptions, the cleanest way to do it is to have the function return the error/success code and take either a reference or pointer argument that gets filled in with the result.

I would not call it an anti-pattern. It's a very well proven workable method that's often preferable to using exceptions.

Answer 9

If you expect your method to fail occasionally, but don't consider that exceptional, I prefer this pattern as used in the .NET Framework:

bool TryMyFunction(out FunctionResult result){    

     //...    
     result = new FunctionResult();
}

Answer 10

Konrad is right, C# uses dual return values all the time. But I kind of like the TryParse, Dictionary.TryGetValue, etc. methods in C#.

int value;
if (int.TryParse("123", out value)) {
    // use value
}

instead of

int? value = int.TryParse("123");
if (value != null) {
    // use value
}

...mostly because the Nullable pattern does not scale to non-Value return types (i.e., class instances). This wouldn't work with Dictionary.TryGetValue(). And TryGetValue is both nicer than a KeyNotFoundException (no "first chance exceptions" constantly in the debugger, arguably more efficient), nicer than Java's practice of get() returning null (what if null values are expected), and more efficient than having to call ContainsKey() first.

But this is still a little bit screwy -- since this looks like C#, then it should be using an out parameter. All efficiency gains are probably lost by instantiating the class.

(Could be Java except for the "string" type being in lowercase. In Java of course you have to use a class to emulate dual return values.)

Answer 11

I agree with those that say this is not an anti-pattern. Its a perfectly valid pattern in certain contexts. Exceptions are for exceptional situations, return values (like in your example) should be used in expected situations. Some domains expect valid and invalid results from classes, and neither of those should be modeled as exceptions.

For example, given X amount of gas, can a car get from A to B, and if so how much gas is left over? That kind of question is ideal for the data structure you provided. Not being able to make the trip from A to B is expected, hence an exception should not be used.

Answer 12

In defense of the anti-pattern designation, this code lends itself to being used in a few ways:

1. Object x = MyFunction().payload; (ignoring the return result - very bad)
2. int code = MyFunction().result; (throwing away the payload - may be okay if that's the intended use.)
3. FunctionResult x = MyFunction(); //... (a bunch of extra FunctionResult objects and extra code to check them all over the place)

If you need to use return codes, that's fine. But then use return codes. Don't try to pack an extra payload in with it. That's what ref and out parameters (C#) are for. Nullable types might be an exception, but only because there's extra sugar baked in the language to support it.

If you still disagree with this assessment, DOWNVOTE this answer (not the whole question). If you do think it's an anti-pattern, then UPVOTE it. We'll use this answer to see what the community thinks.

Answer 13

Joel, I downvoted your answer although I think that this is not an issue that can be settled democratically. Your examples address how this “feature” could be misused – granted. But that's not an argument because every feature could be misused and the same argument could be made against any of these. You claim Nullable is an exception because it's a language feature. I disagree and cite, again, the example of C++ which uses this pattern in its standard library (e.g. in the std::map<>::insert method).

I'll also cite other languages (Python, Haskell) where it's common practice to return multiple values in tuples (that can be seen as anonymous structures). It's actually best-practice to do this. Why not also for status codes? Where's the magical distinction between these cases?

You cite out parameters. I cite Java which doesn't have them for a very good reason. I usually don't even like Java but not having pass-by-reference is one of the smarter decisions. Arguments are meant for input. Output is handled by the return type. If there are multiple values, make the return type contain them. Passing return values via ref arguments is breaking the scheme. I can't think of any argument that would justify breaking the scheme. What's the benefit of ref arguments over return structures? And by the way, there's (usually) no performance benefit.

/EDIT: I revoked my vote. As I mentioned, you can't settle such a dispute democratically and I don't want to encourage it. Additionally, is this the way votes are to be used? Joel mentioned something similar for polls in some podcast but I don't think this platform is well-suited for such polls.

Answer 14

Debates about smells and anti-patterns remind me the "Survivor" TV shows, where you have various programming constructs trying to vote each other off the island. I'd prefer to see "construct X has such-and-so pros and cons", rather than a continually evolving list of edicts of what should and should not be done.