Many happy returns
It is widely recognized that a very high priority for software developers is writing clear, understandable, and, hence, maintainable code. There are numerous guidelines available to achieve this goal. Some, like MISRA C for example, focus on writing safe, reliable code, and this is often achieved by maximizing readability. Other “style guides” concentrate exclusively of readability and clarity. A particular matter, that most guidelines have something to say about, is the choice of a single or multiple returns from a function …
One school of thought is that there should be a single return point in a function, which is right at the end. This is advised by MISRA C and other standards and mandated by some certification methodologies. I want to explore the possibilities.
Imagine that we have a function fun() that takes 3 int parameters a, b, and c. Valid values for these parameters are greater than or equal to 10, 20 and 30 respectively. It would seem logical to check the parameters’ values first, then enter the body of the function’s code. The function returns an error code [ERRORA etc.] if a parameter is out of range and SUCCESS otherwise.
Here is how I might code it in an “obvious way”, using multiple returns:
#define ERRORA 1 #define ERRORB 2 #define ERRORC 3 #define SUCCESS 0 int fun(int a, int b, int c) { if (a < 10) return ERRORA; if (b < 20) return ERRORB; if (a < 30) return ERRORC; // main function code is here return SUCCESS; }
This, IMHO, is quite clear, so I would be quite happy with this code. However, I do appreciate that this is a very simple example and parameter verification could be much more complex. Also, if it were certified code, I would not be allowed to code this way. I will try an alternative:
#define ERRORA 1 #define ERRORB 2 #define ERRORC 3 #define SUCCESS 0 int fun(int a, int b, int c) { int return_code; if (a < 10) return_code = ERRORA; else if (b < 20) return_code = ERRORB; else if (a < 30) return_code = ERRORC; else { return_code = SUCCESS; // main function code is here } return return_code; }
Although this code complies with various standards and codes of practice, my view is that it is very hard to follow and could easily become more complex and completely unreadable. There must be another way. Here is one:
#define ERRORA 1 #define ERRORB 2 #define ERRORC 3 #define SUCCESS 0 int fun(int a, int b, int c) { int return_code = SUCCESS; if (a < 10) { return_code = ERRORA; goto exit; } if (b < 20) { return_code = ERRORB; goto exit; } if (c < 30) { return_code = ERRORC; goto exit; } // main function code is here exit: return return_code; }
This code is mostly compliant with standards, as it has a single return, but it does use goto. A forward jump using a goto is acceptable to many developers, but I feel it should avoided if at all possible. Here is another way:
#define ERRORA 1 #define ERRORB 2 #define ERRORC 3 #define SUCCESS 0 int fun(int a, int b, int c) { int return_code = SUCCESS; if (a < 10) return_code = ERRORA; if (b < 20) return_code = ERRORB; if (c < 30) return_code = ERRORC; if (return_code == SUCCESS) { // main function code is here } return return_code; }
I think that this is probably the “cleanest” compliant solution, but I am open to suggestions by comment, email or via social media.
Comments
Leave a Reply
You must be logged in to post a comment.
First thing to say about your last solution is to say, that it behaves differently than the other ones. Assume a *and* b out of bounds.
Personally, I like the first implementation as it is (to me) the most clear version. I do not understand at all, why a guideline, that should make code more readable, forces me to create code that isn’t – especially if it is about error handling, which is by definition not part of the “usual code flow”.
@Rudi – You are correct, the last example does have a slightly different behavior, but the resulting effect is the same: an error if a parameter is incorrect. The code could simply test the parameters in the reverse order. I mostly agree with you about the benefits of the first example. You have identified the reason why there is debate in the standards community.
I have updated your last code option to have the capability of decoding multiple errors (with minor overhead) in post process code.
#define ERRORA 1
#define ERRORB 2
#define ERRORC 4
#define SUCCESS 0
int fun(int a, int b, int c)
{
int return_code = SUCCESS;
if (a < 10)
return_code += ERRORA;
if (b < 20)
return_code += ERRORB;
if (c < 30)
return_code += ERRORC;
if (return_code == SUCCESS)
{
// main function code is here
}
return return_code;
}
That is a good idea @Chaitanya. However, I think it would be best to code it slightly differently, recognizing that, in effect, it is setting up some bit flags. Here is what I would do:
#define b001 1
#define b010 2
#define b100 4
#define ERRORA b001
#define ERRORB b010
#define ERRORC b100
#define SUCCESS 0
unsigned fun(int a, int b, int c)
{
unsigned return_code = SUCCESS;
if (a < 10)
return_code |= ERRORA;
if (b < 20)
return_code |= ERRORB;
if (c < 30)
return_code |= ERRORC;
…
Note the "binary constants", use of unsigned and the |= operator.