tags:

views:

2371

answers:

10

After programming in C for several years, I realized that I have been ignoring the C convention of returning zero from a function to indicate success. The convention seems semantically wrong to me, as zero is of course false. The problem is I like to name functions like is_valid_foobar(), and in order to accomodate the convention of 'false means success', I would have to be more vague... that is instead of:

if ( ! is_valid_foobar() ) {
    return (error);
}

Other programmers write:

if ( validate_foobar() ) {
     return (error);
}

And my implementation looks like:

int is_valid_foobar (int foobar ) {
     if ( foobar < MAX_ALLOWED ) {
          return TRUE;
      }
      return FALSE;
}

I haven't actually caught any flak for this in code reviews. So I'm thinking it's not such a terrible habit, but it is 'unconventional'. I'm curious what people think.

I'm very careful about the choices I make for function and variable names, and a typical review comment is "the code is really clear", and further it doesn't bother me at all to have to type an extra ! at the front of the function call. But what sayest thou, oh mighty ones of S.O?

+1  A: 

I am not sure I agree that there is a convention of "returning zero from a function to indicate success."

AFAIK, the convention is that zero is false, and all nonzeros are true in the case of predicates.

However, this convention should only apply to obvious predicates that return booleans (e.g., true/false). If the primary goal of a function is not to return a boolean value (e.g., printf, fopen, etc.), then the return value is used to indicate failure reasons, and then you can adopt the UNIX convention of "silence or 0 unless there's a problem".

As for return values, there is nothing wrong with using fixed return values, it makes everything more convenient.

The big risk, IMHO, is that many people declare their own constants, and when others use the same files, you start running into conflicts with their own constants. So your TRUE/FALSE and conflict with somebody else's TRUE/FALSE down the line.

Uri
+1  A: 

That's not so much a C convention as a UN*X shell one. Consider the Standard C iasalpha(), isdigit() etc functions. They all return non-zero to indicate success. Bottom line: in C 'true' is non-zero.

anon
+13  A: 

I'd say both are correct, for different purposes:

If you're performing a simple go/no-go validation, e.g. is_numeric(), then true and false work nicely.

For something more elaborate, the 0==success paradigm is helpful in that it allows more than one error condition to be returned.

In this case, the caller can simply test against 0, or examine the non-0 returns for a more-specific explanation of the failure. e.g. a file open call can fail due to non-existence, insufficient permissions, etc.

Paul Roub
Right - and if you don't like directly using 0 for success, then usually frameworks or platforms provide something like a SUCCESS or (the interestingly named) ERROR_SUCCESS macro/value (along with corresponding error code names) to abstract away from the numeric values somewhat.
Michael Burr
This usage also allows the formulation "if (int e=foo()){/*handle errors*/};", which may look clunky to folks raised on more featurefull languages, but is clean and slick by c standards.
dmckee
+6  A: 

The convention isn't exactly what you seem to think. Processes should exit with a 0 status to indicate success, and functions that return error codes will often use 0 to indicate "no error". But as a boolean, 0 should mean false and nonzero should mean true. To use 0 for boolean true will get you on The Daily WTF someday.

Chuck
+2  A: 

Especially when you name your function is_foo(), the standard C character functions (isdigit(), isupper(), etc) are a good precedent for doing it your way.

IvyMike
+2  A: 

C has no concept of boolean other than 0 being false, and anything else being true.

The idea of returning 0 comes from having a return code indicating what the failure is. If you're not doing a return code, then there's nothing wrong with treating 1 and 0 as true and false. After all, TRUE and FALSE are just typedefs for 1 and 0 in C.

This should be well documented though, and your function comments should say explicity "Returns TRUE or FALSE".

You could also write if (is_invalid_foobar()), that could make the statement easy to understand and still semantically correct.

Costa Rica Dev
A: 

It's only bad if it burns you. If you never trace any bugs to this, I'd say you are OK. What's more important, is the comment above the function that starts with, "Returns zero for..."

The "nonzero is an error" convention is used when attempting to say why along with the error flag.

gbarry
+2  A: 

It's been a while since I programmed C, but I think you're talking about two different kinds of functions here:

  • is_foo is a functions that checks for some property foo and returns 0 (false) or non-0 (true) to indicate the boolean value of that property. When calling those functions no error state is expected (and if it happens, it's mapped on one of those values).
  • foo is a function that executes the action foo. It returns 0 on success and a non-0 value on error.
Joachim Sauer
A: 

to me, actually checking against 0 seems more natural and the compiler optimizes it anyway so i like to write for instance

if ( check_foo == 0 ) //do something

A: 

Return value by a function -- conventions for True and False OR Success and Error:

  1. An API etc. shows success with return value 0 and failure with return value = non zero error value.
  2. If something is True; functions return 1. E.g. isStackEmpty() will return 1 if stack is empty.
S3