The Design of Software (CLOSED)

A public forum for discussing the design of software, from the user interface to the code architecture. Now closed.

The "Design of Software" discussion group has been merged with the main Joel on Software discussion group.

The archives will remain online indefinitely.

proper error handling

I need to write a function (in C) that reads an integer parameter from a file, and that has a mechanism of telling if something went wrong. Since any int is legal, assigning special value to the returned number is not an option. Thus I have the following options:
Option A
int readParam(const char* iParamName, bool& iStatus), and then call it like that:
bool status;
int param=readParam("BLAH",  status);

Option B
bool readParam(const char* iParamName, int& iParamVal), and then call it like that:
int param;
if(readParam("BLAH", param){

Which way is better.
Thanks Send private email
Monday, May 02, 2005
There are no hard and fast rules for this kind of thing. The second approach has the advantage that you needn't declare a variable for the status result, which you're probably going to want to check then discard. So it's a natural fit for putting in the condition part of an if() statement, which is dead common in C.

On the other hand, if there's something documented that you can do up front to prevent readParam from failing, the first may be preferred, but with the tweak of having status as a bool * defaulted to 0. You can then do something like "int param=readParam(str)" having first guaranteed str is valid, without having to clutter the code with the needless error fluff.

Generally I vaguely prefer the first if there's a significant chance of error, but the second approach shouldn't be avoided, because it cuts down on the number of temporary variables you are obliged to have.
Monday, May 02, 2005
I definitely feel more comfortable with the second style.  The function returns the success, rather than the value read itself..

Only small thing to ensure is that the your readParam function doesn't trash the parameter if it fails - whether it zeros it, or doesn't modify it except upon success or whatnot, just make sure the behaviour is documented!
i like i
Monday, May 02, 2005
What about:
struct Pair {
  int value;
  bool status;
struct Pair retVal=ReadParameter("param");
Monday, May 02, 2005
I like:

int param= 0;
Status rc= readParam("BLAH", param);

if (rc != OK)
  return FAIL;

I don't like embedding calls in if statements. The results aren't loggable because you don't have access to the values.

I don't like if (!var) type statements because it has to be thought about whereas my version reads like a story. I don't like using bool for a return status because what to true or false mean returned from readParam?
son of parnas
Monday, May 02, 2005
Yup, what 'son of parnas' said.

The returning of an 'int' indicating success or failure is a common 'C' pattern.  I agree with parnas when he says put the return value in a variable for later error reporting.  And I agree you should ALWAYS have an explicit check against a value for success.

Unfortunately, in the Standard C Library it is NOT standard whether to return a zero for success, a positive number for success, a negative error code for failure, or a negative 1 for success.  Different calls use different standards.

However, in YOUR library you can set a standard for returns and stick with it.

Returning an error parameter is very un-common.  I've been known to define for my calls a 'default' value, and return that with a negative number indicating some error condition.

Another common 'C' idiom is the global 'errno', so you could emulate that approach with your own globals approach -- return a 0 to indicate failure, and the caller then knows to check some global for what really went wrong.
Monday, May 02, 2005
IIRC using Val or whatever it was called in Turbo Pascal would not only tell you that the parse of the int failed, it would tell you where and why.

And from that, apps would make useful error messages such as:

balance: $102.3445c
                  ^ not a number

(I just know that a lack of a <pre> is going to kill my alignment, but you get the idea)
i like i
Monday, May 02, 2005
The traditional meaning for 'true' and 'false' are 'success' and 'failure' respectively. This is due to the traditional connotations of 'true' and 'false'. (For ints, of course, 0 is success -- that's because ints aren't the same as bools.)

Most of the time, code can't be written to recover from errors automatically, and in this situation anything particularly fine-grained becomes pointless. All the checks against OK and the vast list of error enums (or worse, #defines) are just pointless noise in the program. Far better not to bother, and spend the time making it easy for the person sat in front of the PC to track down the error. You can do this by printing messages to the screen somehow, or saving them in a buffer that is displayed if there's a problem.

My slightly contentious advice should be ignored by anyone who feels it does not apply to their situation. You can tell if it applies by looking through code to find out how many times the error checks boil down to if succeeded/if failed. Then consider whether the existing scheme is a help or a hindrance. That's how I came to prefer this approach for the situations where it is appropriate.
Monday, May 02, 2005
> I agree with parnas when he says put the return value in a
> variable for later error reporting.

And for inspecting in the debugger.

Monday, May 02, 2005
Or, of course, you can do your own damn homework.
John Haren Send private email
Monday, May 02, 2005
Thank you all for your helpful input. I think I'll stick to AllanL5's suggestion. The main reason for this is that it would result in a pretty elegant solution, whitout too much rewriting someone else's code. I didn't mention that, but I'm trying to introduce error handling to a library that completely ignored it (they said they trusting their inputs). Allan's suggestion will enable me to introduce error handling without breaking already working software.
John Haren: I finished with all my homeworks couple of years ago. Send private email
Monday, May 02, 2005
Tom_, "(For ints, of course, 0 is success -- that's because ints aren't the same as bools.)"


if(mybitflags & INTERESTING_FLAG) { ...

if(someptr) { ...
i like i
Tuesday, May 03, 2005
Yeah, I'm sure -- well, pretty sure! What you've done there is, in effect, request that the int be interpreted as a bool, with 0 being false and anything else being true. But that's only because you've asked for it. (You can also do this by casting to bool explicitly, casting to bool implicitly, using another type of conditional statement, or using the ||, && and ! operators. I think that's all of them.)

Normally ints have no innate interpretation.
Tuesday, May 03, 2005
So Tom, if every time you do a boolean-ish operation on an int it is implicitly reinterpretted as a bool, then int itself would never have a concept of "0 = success"?

0 might be a convention.  Many C functions use negative returns to denote various error conditions (or flag that you should check errno - one of the dumbest things I think).

But just convention?

I know that if you reinterpret a bool as an int, you get strictly 0 (false) or 1 (true).  Very useful trick to do stuff like "!!someint" to get strictly 1 or 0 from it..
i like i
Wednesday, May 04, 2005
Yes. Just convention. There are, really, two sets of values to return -- those that indicate success, and those that indicate failure. The set of success values generally has 1 element, because you don't usually care how something succeeded, just that it did. Failure values -- well, often there's just one, but there can be more, so there's always at least as many as there are success values.

Assuming there is 1 success value and more than 1 failure value, you could use any integer for the success value, and any set of integers for the failure values. But 0 is thought of differently, so it makes a handy single-item set (that you can map to your single-item set of success values), and people seem to intuitively get this.

But there's nothing intrinsic in (int)0 to mean success! Or failure, for that matter. In fact, there's nothing intrinsic in (int)0 at all. It's just one of ( 1 << ( sizeof( int ) * CHAR_BIT ) ) possible integer values, and is no more and no less than what you make of it.

You could of course say the same of bools, but the naming ("true" and "false") makes a particular interpretation ("true" = "success", "false" = "failure") more likely. And that's kind of what I'm getting at -- once you turn an int into a bool, it stops being just one of the vast possible different ints you can have, and turns into something that is "true" or "false".

(The above does not claim to be awe-inspiring or well thought out! I don't claim that either.)
Wednesday, May 04, 2005

This topic is archived. No further replies will be accepted.

Other recent topics Other recent topics
Powered by FogBugz