Opinions on coding style re: error handling

[tggzzz]

The only take home for me from that was sequence diagrams which are a good communication tool for protocol descriptions.

The FSM diagrams are OK, but they are a ripoff of Harel's StateCharts.

Revealingly, when designing the Boeing 777 systems in the early 80s, it was realised there was a need for two parallel hierarchies:

• one for specification, and it could be executed
• one for implementation

They are independent, since one part of  the specification can be implemented in hardware and/or software and/or mechanics.

The only part they automated was a database containing the cross references between the design and implementation hierarchies, so they could see

• all specs had been implemented and where
• for any implementation artifact, which parts of the specification it was implementing

[AndyC_772]

Thanks for the suggestions so far, everyone! Some good ideas here.

For me personally:
- readability, a maze is easier to solve when there is only one exit.
- if you need multiple returns than it can be a signal that your function is overly complex, eg can't you better break it up in multiple functions ?
- if you need to clean things up before exiting, for instance you allocated memory you have to free, you have to write that cleanup code for each and every return.

I'm sure I've been taught the 'only one return' "rule", the justification being that if there's only one way to exit from a function, then it's easy to see that the function is always going to carry out whatever clean-up operations are needed prior to that exit.

I'm not convinced writing the code that way is necessarily a good thing, though. In this particular case, for example, the steps to be performed are basically:

- Trigger an external device to generate some raw data, which should result in that data ending up in a queue in RAM
- If, and only if, that data is received OK, then check it's valid
- If, and only if, that data is valid, then calculate a useful result from it

The exit conditions aren't necessarily quite the same in each case, so generic clean-up code might still have to act differently depending on what the failure was. For example, if the external device doesn't generate the expected data, we probably don't want to advance the queue, but we do in every other case.

Maybe another option is to set flags before each potential exit point, which describe the things that need doing, eg.

Code: [Select]

uint32_t exit_actions = 0;

if (fetch_data() != RESULT_FETCH_OK)
{
  exit_actions |= FLAG_REBOOT_DATA_SOURCE;
  exit_code = FAIL_NO_DATA;
  goto exit_point;
}

if (data_valid() != RESULT_VALID)
{
  exit_actions |= FLAG_ADVANCE_QUEUE | FLAG_RECALIBRATE;
  exit_code = FAIL_DATA_INVALID;
  goto exit_point;
}

if (process_data() != RESULT_WITHIN_LIMITS)
{
  exit_actions |= FLAG_ADVANCE_QUEUE;
  exit_code = FAIL_DATA_OUTSIDE_LIMITS;
  goto exit_point;
}
.
.

exit_point:

if (exit_actions & FLAG_REBOOT_DATA_SOURCE)
{
  reboot_data_source();
  flush_everything();
  log_major_error();
}

if (exit_actions & FLAG_ADVANCE_QUEUE)
{
  advance_queue();
}
.
.
return exit_code;

[Kjelt]

I'm not convinced writing the code that way is necessarily a good thing, though.

Neither am I and the programmers community is very devided on this topic AFAIK.
Most young players I encounter only have knowledge of program languages that have automatic garbage collection so that is a totally different story.

In this particular case, for example, the steps to be performed are basically:

- Trigger an external device to generate some raw data, which should result in that data ending up in a queue in RAM
- If, and only if, that data is received OK, then check it's valid
- If, and only if, that data is valid, then calculate a useful result from it

I would split this up in seperate functions.
Func 1: trigger device and get data
Func 2: check received data for completeness
Func 3: Process data

I would probably put it in a statemachine, that way the transition conditions are even more clear.

But each programmer has its own style and preferences, in that sense we are artists

[bd139]

Unfortunately it's my job to stop the artists painting with their own excrement

[IanB]

I'm not convinced writing the code that way is necessarily a good thing, though. In this particular case, for example, the steps to be performed are basically:

- Trigger an external device to generate some raw data, which should result in that data ending up in a queue in RAM
- If, and only if, that data is received OK, then check it's valid
- If, and only if, that data is valid, then calculate a useful result from it

The exit conditions aren't necessarily quite the same in each case, so generic clean-up code might still have to act differently depending on what the failure was. For example, if the external device doesn't generate the expected data, we probably don't want to advance the queue, but we do in every other case.

In that case I would do the clean up locally after each operation, then check whether it is OK to continue, then "goto exit" if not. In this case "goto exit" does nothing except jump to the single return at the end of the method.

I do like the "goto exit" pattern rather than having extra returns interspersed within a method. This helps enormously with logging and debugging (for instance you can reliably put a trace log or a break point in one place before the method returns and be sure it always gets encountered).

[IanMacdonald]

I would prefer using multiple exit points rather then having a situation where the process runs on though multiple error tests, any one of which might fail to trap the problem and allow something bad to happen. Basically, an abort should be an abort.

You can always call a cleanup function before each return. 

As for GOTO, I'm currently writing code using javascript callbacks, and that is just oh-so like vintage BASIC. Only worse, since at least after a GOTO you can GOTO back where you came from. Perfectly manageable if you are careful about keeping track of where your branches are.

With callback functions you can't even go back where you came from, because the callback can only be to a function. So whatever process was executing when the need for the callback arose has to be started over afresh. Needless to say there are plenty of opportunities for creating endless loops with that scenario. Which is typically what's involved when a website displays the 'script has stopped responding' message. It likely hasn't stopped running as such, but is stuck in an endless series of callbacks.

It's been said that if you wait long enough, all bad ideas will be resurrected. 

[tggzzz]

It's been said that if you wait long enough, all bad ideas will be resurrected. 

Indeed, I've seen that happen too many times.

Classic examples are

• becoming sick of unnecessary errors discovered at runtime in dynamically typed languages, so moving to errors caught by the compiler in statically typed languages. Then the next generation feeling constrained by statically typed languages (and disliking typing control-space in IDEs) and moving to dynamically typed languages
• asynchronous protocols layered on top of synchronous protocols layered on top of synchronous protocols layered.... it is turtles all the way down
• graphical programming languages that are supposed to make it less necessary to use the underlying textual language (today's example is visduino)

[Howardlong]

I haven't used a goto in a high level language since BASIC with line numbers was a thing. I also tend to avoid returning from a function prematurely. I even make efforts to try to avoid 'continue' and 'break' inside loops. Those last points are more of a value judgement, if it's going to make things more convoluted and less readable by avoiding them, then I go for the most maintainable solution. It is rare though to see a return other than at the end of a function, or a break within a loop, and even more rare a continue in my code (I can't remember the last time I used continue).

While I'm not particularly dictatorial about it to others, my own C code follows a few different methods for dealing with exceptions, depending on the complexity and scenario, but I always use braces for single line blocks, and always consistently follow the same white space/indent methodology. At this stage in my career, and being a one man band mostly working on my own projects, I almost always get to decide how to write code. I realise not everyone has that luxury.

I have little trouble reading others' code in whatever format they choose, as long as it's well laid out and consistent. Frankly IMHO if you get upset reading someone else's reasonably formatted and structured code, then you need to practice more!

One of the nice things about lower level headless embedded stuff (about 70% of what I do programming-wise) is that a good deal of errors are fatal, and generally you will avoid memory and resource allocation errors as everything tends to be statically allocated. Catching all errors is good practice in the debugging cycle, even if it simply generates a CPU exception where you can figure out where it was generated. In a finished retail product, a good deal of the time you should never get there.

Dealing with the OP, within the scope of a medium to complex function, I include an initialised local "rc" variable (Return Code, often set up as an a global enumerated typedef) and test rc at each functional block in the function. Also at function scope I'm likely to have a selection of handles and pointers, that are initialised to null, but are subsequently set up as the function progresses; alternatively they may be within a structure passed by reference to the function.

At the end of the function, just prior to returning, if rc contains an error, I deallocate/dispose/terminate/reset any allocated resources by testing each of the handle and pointer values that were set in the reverse order they were allocated. I also reset to null/invalid each handle and pointer as they're deallocated/terminated etc. The rc is then returned to the caller, and that can choose to do its own error handling based on that.