views:

188

answers:

7

Hello,

I have observed that the general coding convention for a successful completion of a method intended functionality is 0. (As in exit(0)).

This kind of confuses me because, if I have method in my if statement and method returns a 0, by the "if condition" is false and thereby urging me to think for a minute that the method had failed. Of course I do know I have to append with a "!" (As in if(!Method()) ), but isn't this convention kind of self contradicting itself ??

+2  A: 

The convention isn't contradicting itself, it's contradicting your desired use of the function.

One of them has to change, and it's not going to be the convention ;-)

I would usually write either:

if (Function() == 0) {
    // code for success
} else {
    // code for failure
}

or if (Function() != 0) with the cases the other way around.

Integers can be implicitly converted to boolean, but that doesn't mean you always should. 0 here means 0, it doesn't mean false. If you really want to, you could write:

int error = Function();
if (!error) {
    // code for success
} else {
    // code for failure, perhaps using error value
}
Steve Jessop
+11  A: 

You need to differentiate between an error code and an error flag. A code is a number representing any number of errors, while a flag is a boolean that indicates success.

When it comes to error codes, the idea is: There is only one way to succeed, but there are many ways to fail. Take 0 as a good single unique number, representing success, then you have every other number is a way of indicating failure. (It doesn't make sense any other way.)

When it comes to error flags, the idea is simple: True means it worked, false means it didn't. You can usually then get the error code by some other means, and act accordingly.

Some functions use error codes, some use error flags. There's nothing confusing or backwards about it, unless you're trying to treat everything as a flag. Not all return values are a flag, that's just something you'll have to get used to.

Keep in mind in C++ you generally handle errors with exceptions. Instead of looking up an error code, you just get the necessary information out of the caught exception.

GMan
+1 for mentioning exceptions.
Etienne de Martel
A: 

There are various conventions, but most common for C functions is to return 0 on failure and a positive value on success so you can just use it inside an if statement (almost all CPUs have conditional jumps which can test whether a value is 0 or not which is why in C this "abused" with 0 meansing false and everything else meaning true).

Another convention is to return -1 on error and some other value on success instead (you especially see this with POSIX functions that set the errno variable). And this is where 0 can be interpreted as "success".

Then there's exit. It is different, because the value returned is not to be interpreted by C, but by a shell. And here the value 0 means success, and every other value means an error condition (a lot of tools tell you what type of error occurred with this value). This is because in the shell, you normally only have a range of 0-127 for returning meaningful values (historic reasons, it's a unsigned byte and everything above 127 means killed by some signal IIRC).

DarkDust
"most common for C functions is to return 0 on failure and a positive value on success"... You mean like in `printf` and `read`? In those cases, the result is the amount of data processed. Are there cases where the result isn't a byte count or similar, but has a >0 return value for success?
Mike DeSimone
No, I mean most "user" level functions that return a boolean value or a pointer. In both cases, 0 is false/failure, everything else is true/success (getting a valid pointer is a success to me ;-). At least in most C projects I've worked with this was the case, YMMV. As I said, almost all POSIX functions like `read`, `printf`, `stat`, `open`, etc. return -1 on error.
DarkDust
+2  A: 

You have tagged your question as [c] and [c++]. Which is it? Because the answer will differ somewhat.

You said:

I have observed that the general coding convention for a successful completion of a method intended functionality is 0.

For C, this is fine.

For C++, it decidedly is not. C++ has another mechanism to signal failure (namely exceptions). Abusing (numeric) return values for that is usually a sign of bad design.

If exceptions are a no-go for some reasons, there are other ways to signal failure without clogging the method’s return type. Alternatives comprise returning a bool (consider a method try_insert) or using an invalid/reserved return value for failure, such as string::npos that is used by the string::find method when no occurrence is found in the string.

Konrad Rudolph
Its perfectly valid to not use exceptions and use return types with C++...
Goz
@Goz: That’s what I wrote. But using numeric error codes is just insane. Type safety, anyone? And what do you do with the real return value (provided there is one)? Using a reference argument? Nonono. Bad design. In a design review I would unconditionally reject numeric error codes.
Konrad Rudolph
Also, in C++, if you simply want to return "success" vs. "failure" (e.g. `empty()`), you return `bool`. If you want to return a result code, where there is one case of "success" and several of "failure", but you don't want to break control flow like an exception would (e.g. you're just going to forward the result code somewhere else, say to a log), return `int` (a result code) where `== 0` is success and `!= 0` is failure. If you have "fault" cases (i.e. can't work as intended but can work in a degraded fashion), then return `int` where `== 0` is success, `>0` is fault, and `<0` is failure.
Mike DeSimone
@Konrad: Actually, using reference arguments for output and returning a result code (or something like it) is the C++ flavor of standard C design. See the POSIX `strtol()` function. Also, I'm not sure what the type safety issue you refer to is with result codes. I think you meant namespace issues (i.e. two different errors can have the same code).
Mike DeSimone
@Mike: two things. First, nobody can claim that `strtol` is a C++-style function. This isn’t how good C++ APIs are designed. That’s the classical example of a legacy C API that incidentally also gets used in C++. Secondly, the issue of type safety. C++ has an extensible type system. You create custom classes to refer to custom entities. Using integers to refer to errors is a design error. If you want to represent an error, write a class for it. `enum` may be OK for very constrained solutions, but generally they aren’t well-suited either. All this is really API design 101.
Konrad Rudolph
@Konrad: Mike is correct, there is no problem with returning an error code (using an enum is my preference) and having values returned in references in a c++ method. This is common practice and good style.
gbrandt
"C++ has an extensible type system. You create custom classes to refer to custom entities. Using integers to refer to errors is a design error." ... I read that and think, "so if I represent a temperature as a `float`, is that a design error? Or is it just me not wanting to create unpaid overtime for myself?" There's a leap you're making between "you can define classes" and "you should do so at every opportunity" that I'm not buying into. In cases where exception handling isn't appropriate, what is *always* special about errors that demands new class definitions?
Mike DeSimone
A: 

exit(0) is a very special case because it's a sentinel value requesting that the compiler tell the operating system to return whatever the real success value is on that OS. It's entirely possible that it won't be the number 0.

As you say, many functions return 0 for success, but they're mainly "legacy" C library OS-interfacing functions, and following the interfacing style of the operating system's on which C was first developed and deployed.

In C++, 0 may therefore be a success value when wrapping such a C legacy interface. Another situation where you might consider using 0 for success is when you are effectively returning an error code, such that all errors are non-zero values and 0 makes sense as a not-an-error value. So, don't think of the return value as a boolean (even though C++ will implicitly convert it to one), but as an error code where 0 means "no error". (In practice, using an enum is typically best).

Still, you should generally return a boolean value from functions that are predicates of some form, such as is_empty(), has_dependencies(), can_fit() etc., and typically throw an exception on error. Alterantively, use a sub-system (and perhaps thread) specific value for error codes as per libc's errno, or accept a separate reference/pointer argument to the variable to be loaded with the error code.

In C, it's entirely possible that `NULL` might not be defined as `((void*)0)`, but I have yet to see an OS that didn't have it as 0. IIRC, C++ defines a null pointer as `0` instead of `NULL`.
Mike DeSimone
DarkDust
@Mike: C++ doesn't do that. It defines `NULL` as zero (not necessarily `0`, the type need not be `int`) Neither 0 nor `NULL` is a null pointer, but both are null pointer _constants_ .
MSalters
A: 
int retCode = SaveWork();

if(retCode == 0) {
  //Success !
} else if(retCode == ERR_PERMISSIONS) {
  //User doesn't have permissions, inform him
  //and let him chose another place
} else if(retCode == ERR_NO_SPACE) {
  //No space left to save the work. Figure out something.
} else {
    //I give up, user is screwd.
}

So, if 0/false were returned to mean failure, you could not distinguish what was the cause of the error. For C++, you could use exceptions to distinguish between different errors. You could also use a global variable, akin to errno which you inspect in case of a failure. When neither exceptions or global variables are desired, returning an error code is commonly used.

nos
Woah. Make that a `switch`!
sbi
A: 

As the other answers have already said, using 0 for success leaves everything non-zero for failure. Often (though not always) this means that individual non-zero values are used to indicate the type of failure. So as has already been said, in that situation you have to think of it as an error code instead of an success/failure flag.

And in that kind of situation, I absolutely loathe seeing a statement like if(!Method()) which is actually a test for success. For myself too, I've found it can cause a moment of wondering whether that statement is testing for success or failure. In my opinion, since it's not a simple boolean return, it shouldn't be written like one.

Ideally, since the return is being used as an error code the author of that function should have provided an enum (or at least a set of defines) that can be used in place of the raw numbers. If so, then I would always prefer the test rewritten as something like if(Method() == METHOD_SUCCESS). If the author didn't provide that but you have documentation on the error code values, then consider making your own enum or defines and using that.

If nothing else, I would write it as if(Method() == 0) because then at least it should still be clear to the reader that Method doesn't return a simple boolean and that zero has a specific meaning.


While that convention is often used, it certainly isn't the only one. I've seen conventions that distinguish success and failure by using positive and negative values. This particularly happens when a function returns a count upon success, but needs to returns something that isn't a value count upon failure (often -1). I've also seen variations using unsigned numbers where (unsigned int)-1 (aka 0xffffffff) represented an error. And there are probably others that I can't even think of offhand.

Since there's no single right way to do it, various authors at various times have invented various schemes.

And of course this is all without mentioning that exceptions offer (among other things) a totally different way to provide information when a function has an error.

TheUndeadFish