8

What are the best practices for writing C or C++ functions that return an int that represents a status code?

Specifically, I want to know about the client usage but other tips are welcome.

For example, can I write something like this:

int foo() {
  return 0;  // because everything was cool
}

And then use it like this?

if (foo()) {
  // what to do if false, e.g. non-zero, e.g. not OK
} else {
  // what to do if true, e.g. zero, e.g. OK
}

This should work because best practices typically dictate that a status code of 0 means everything was OK and also 0 means false in a boolean statement.

However, this wouldn't be good, right:

if (!foo()) {
  // what to do if true
} else {
  // what to do if false
}
Jason Marcell
  • 2,785
  • 5
  • 28
  • 41
  • 9
    `0` does not mean `true` in a boolean statement. – Chad Sep 16 '11 at 20:57
  • 1
    You may want to look into exception handling in C++, which was partly designed to eliminate this style of coding. – templatetypedef Sep 16 '11 at 20:57
  • @Chad: he seems aware of that – Mooing Duck Sep 16 '11 at 20:58
  • I would say best practice is `if ( foo() == 0) { //good state`. I've heard you should prefer the most common case to be the first option in an if/else chain. – Mooing Duck Sep 16 '11 at 21:00
  • `0` doesn't mean true in C. It does in `sh`, though, which is why so many programs return `0` from `main` to indicate success – Pillsy Sep 16 '11 at 21:01
  • Thanks @Chad. I fixed my typo. Please check again to make sure I got it correct this time. I always confuse that. – Jason Marcell Sep 16 '11 at 21:04
  • @Mooing Duck, I agree, but it seems people like to return 0 for success and 0 is interpreted as false, therefore it seems that the language (plus the custom of 0 == success) wants you to do as okorz001 has shown. – Jason Marcell Sep 16 '11 at 21:08
  • @Pillsy sh's true and false are inverted from C. 0 is the only true value, and anything else is false. – Oscar Korz Sep 16 '11 at 21:35
  • @okorz001 didn't you just get that backwards? 0 is false; everything else is true. 0 however is success and all else is failure. – Jason Marcell Sep 16 '11 at 21:48
  • @templatetypedef, IMHO try/catch is a horrible way to do error handling. http://www.joelonsoftware.com/items/2003/10/13.html – Jay Sep 16 '11 at 21:54
  • @Jay: That article is an oft-linked terrible argument against exceptions. Arguments about exceptions died several years ago: they win. They just require a paradigm shift, and the only people that argue against them are those that cannot or refuse to change their practices. They're also faster: http://lazarenko.me/tips-and-tricks/c-exception-handling-and-performance – GManNickG Sep 16 '11 at 23:10
  • @Jason M I am referring to sh's (and thus bash's) conditionals. An if branch is only taken if the exit code is 0. Otherwise the else branch is taken (if it exists). Check the exit codes of `/bin/true` and `/bin/false`. – Oscar Korz Sep 17 '11 at 02:13

7 Answers7

11

We use this in C where I work:

int err = foo();
if (err) {
    // armageddon
}

The assignment and if could be combined, but with more complicated function calls it gets more confusing and some people are confused by assignment in a conditional (and gcc hates it).

For C++, I would prefer exceptions if available, otherwise the above.

Edit: I would recommend returning 0 on success and anything else on error. This is what unix command line utilities do.

Oscar Korz
  • 2,457
  • 1
  • 18
  • 18
  • 1
    // And armageddon sick of it, too. :-) – Andy Finkenstadt Sep 16 '11 at 21:00
  • 6
    I would add that the **reason** it's usually best to return 0 on success and nonzero on error (as opposed to boolean 1/0 meaning true/false) is that there are usually many reasons an operation can fail but only one type of success. – R.. GitHub STOP HELPING ICE Sep 16 '11 at 21:52
  • 2
    In and of itself, that doesn't matter. The significance of 0 is that programmers are lazy and this allows the terse `if(foo())` to work. Success could just as easily be 42, and we could all write `if (foo() != 42)` – Oscar Korz Sep 17 '11 at 02:09
5

If you really want to use status codes that way, use them with an enum or block of #define statements that describe the intention of the status code.

For example:

enum
{
   kSuccess = 0,
   kFailure = -1,
}

function foo()
{
    return kSuccess;
}

if (kSuccess == foo())
{
    // Handle successful call to foo
}
else
{
    // Handle failed call to foo
}

This way, the intention is clear and there's no error-prone guesswork when someone wants to use or maintain your code in the future.

adpalumbo
  • 3,031
  • 12
  • 12
  • It's just a style thing, used to help constant values stand out from variables and other identifiers. – adpalumbo Sep 16 '11 at 22:06
  • @adpalumbo: But why do they need to stand out? :) – GManNickG Sep 16 '11 at 23:11
  • Is it akin to Hungarian notation? Or is "k" an initial for your library/project/company? Or..? – R.. GitHub STOP HELPING ICE Sep 16 '11 at 23:41
  • It appears in a lot of places: it's used in Hungarian notation, Apple's naming convention (which is not Hungarian), and many others. The real reason it appears in this sample is simply because it's used in my current employer's style guide, so its second nature for me now. – adpalumbo Sep 16 '11 at 23:51
3
if (foo()) {
  // what to do if false
} else {
  // what to do if true
}

The problem with this approach is excess nesting. Suppose you have three functions you want to call:

if(foo1()) {
    if(foo2()) {
        if(foo3()) {
            // the rest of your code
        } else {
            // handle error
        }
    } else {
        // handle error
    }
} else {
    // handle error
}

To solve the excess nesting problem, invert the return value:

if(!foo1()) {
    // handle error
    return;
}

if(!foo2()) {
    // handle error
    return;
}

if(!foo3()) {
    // handle error
    return;
}

This solution suffers from another problem. It mixes the program logic with the error handling code. This complicates everything. Ideally, you want the program logic and error handling separated. This problem can be fixed with the goto

if(!foo1()) 
    goto error1;

if(!foo2())
    goto error2;

if(!foo3())
    goto error3;

return;

error1:
    // handle error
    return;
error2:
    // handle error
    return;
error3:
    // handle error
    return;

Much cleaner.

Also, the goto can solve the problem of resource deallocation. See Using goto for error handling in C by Eli Bendersky for more details.

Jay
  • 9,314
  • 7
  • 33
  • 40
  • Wait, but "not"-ing something non-zero does not necessarily make it zero, right? – Jason Marcell Sep 16 '11 at 21:37
  • "the result is 1 if the operand is 0, and the result is 0 if the operand is not 0." http://c.comsci.us/etymology/operator/logicalnot.html – Jay Sep 16 '11 at 21:47
  • Yes, but since there is no first-class Boolean type in C, isn't the ! just an arithmetic negation on the integer operand? It's not a logical negation, right? It just flips the bits. – Jason Marcell Sep 16 '11 at 21:56
  • so "not"-ins something non-zero does necessarily make it zero – Jay Sep 16 '11 at 21:56
  • !10 is equal to 0, the article explains this – Jay Sep 16 '11 at 21:58
  • btw, if you want actual boolean literals, there are a few options. See http://stackoverflow.com/questions/1921539/using-boolean-values-in-c – Jay Sep 16 '11 at 22:05
  • You're confusing the not operator "!" with the bitwise not operator "~". They are two separate things. The "~" operator has the effect you're thinking of. – Jay Sep 16 '11 at 22:16
  • @Jason: C has a boolean type, in `stdbool.h`. – GManNickG Sep 18 '11 at 06:20
  • @Jay: you should not do this with the resource deallocation. The crucial point is to have a single return for multiple error cases. imagine you have three resources allocated - one after every if you want to deallocate them in reverse order you would have three labels. then you would jump to the case where you deallocate all resources if you fail in the last if. and if you have to abort earlier you'd just deallocate 2 resources and jump to the penultimate label a.s.o. if you always want to deallocate all resources just skip the return in the middle. – Alexander Oh Sep 18 '11 at 09:18
  • @GMan, thanks for pointing out `stdbool.h`. I had never heard of that before. However, I'll still point out that it just uses macros and also that this is part of a library and not the core language, so I might still argue that C does not have a boolean type. Granted, it is a "standard" library, so I guess you can count on it being there. So that's just about as good as it being part of the core language, right? – Jason Marcell Sep 19 '11 at 03:14
  • @Jason: Sure. This statement is true: C has a boolean type. How you use that type is not a concern, but you are guaranteed that it's there in a well-defined manner. 'Core' or 'library', I think, is only a categorization *within* the language; note, then, that to even ask if it's part of the core or of the library, we already have to agree it's, at least, part of the language itself. – GManNickG Sep 19 '11 at 03:18
1

The return statuses should be defined in your interface and known to the caller. Some return 0 on failure (because it's easy to check with !), some return 0 on success (because they have enum of error codes, with OK being the first item).

There's no law or standard, each interface defines its own conventions. In C++ - use exceptions.

littleadv
  • 20,100
  • 2
  • 36
  • 50
1

Best practice is to document your code so that yourself and others can quickly look up what the return codes will be when doing error checking.

Timulus
  • 334
  • 3
  • 12
0

Just jumping on board with another option that may be appropriate in your circumstances:

enum fooret { GOOD, BAD, UGLY, WORSE };

fooret foo();  // defined elsewhere

switch(foo())
{
case BAD:
case UGLY:
   // maybe a recoverable failure(s)...
   // take appropriate actions
   break;
case WORSE:
   // maybe non-recoverable
   break;
case GOOD:
   // successful, take appropriate actions
   break;
}
Chad
  • 18,706
  • 4
  • 46
  • 63
-2
int foo() {
   try{
    ...
   return 1
   }
   catch
   {
   return 0;  // because everything was cool
   }
}

I would start by wrapping everything in a try/catch block. Also instead of using and int it might make more scene to return a Boolean value. This is just a little more intuitive when testing in the if statement.

JBone
  • 3,163
  • 11
  • 36
  • 47
  • Swallowing exceptions is bad practice, for lots of reasons: http://www.google.com.ar/search?sourceid=chrome&ie=UTF-8&q=swallowing+exceptions – dario_ramos Sep 16 '11 at 21:03