11

I am working on embedded program and in certain cases if a condition is not meant, I would like to return from function as quickly as possible. if I have the following code and I am doing embedded programming:

foo() {
   if (a < b) {
       return 0;  // bail, since condition is met
   } else {
       // lots of calculations in this block
   }
   return 1;
}

My question is, is it bad having multiple return statements? Is it bad practice? Are there better methods? Does MISRA say anything about it?

NOTE: This question is particular to Embedded Systems, has to do with MISRA not just C/C++

Thanks...

user1135541
  • 1,781
  • 3
  • 19
  • 41
  • 10
    No, it's definitely not bad practice. – chris Jun 18 '13 at 19:30
  • 20
    I'd also go as far as removing the `else`. It's not necessary. – 000 Jun 18 '13 at 19:31
  • 1
    Feels like this would be better on programmers.SE. – ardent Jun 18 '13 at 19:34
  • once you know it is better to not abuse , use it as you need – Luca Rocchi Jun 18 '13 at 19:40
  • [Related answer regarding multiple returns and similar constructs](http://stackoverflow.com/questions/10975722/why-continue-is-considered-as-a-c-violation-in-misra-c2004/10995589#10995589). – Lundin Jun 18 '13 at 20:06
  • +1 for the specific question about MISRA, which seems to distinguish it from the typical "what's your opinion?" question. – Adrian McCarthy Jun 18 '13 at 20:08
  • Btw this function could as well have been written as `bool foo (void) { return a >= b; }`, remaining perfectly clear as well as MISRA-compatible. – Lundin Jun 18 '13 at 20:20
  • 2
    @Lundin: But then `lots of calculations in this block` doesn't happen. – Bill Jun 18 '13 at 20:25
  • @JoeFrambach: +1 since this is one of my main reasons to use multiple returns: reduce code depth. – eckes Jun 18 '13 at 20:41
  • @Bill I didn't suggest an optimization, I suggested a readability improvement, which would also sate MISRA. – Lundin Jun 19 '13 at 07:41
  • 2
    @Lundin: You suggested a transformation which changes the semantics of the function. Instead of doing lots of stuff if `a >= b`, and then returning, you just `return`. – Bill Jun 19 '13 at 15:18

5 Answers5

18

MISRA requires a single return statement:

(MISRA, rule 14.7 : required) "A function shall have a single point of exit at the end of the function"

Now, personally I don't think it is a good rule. Minimize the number of return statements but use a return statement when it enhances the readability of your code.

For example guard clauses can make your code cleaner and more readable.

I suggest you to read this article about duffing (writing code from top to bottom):

ouah
  • 142,963
  • 15
  • 272
  • 331
  • That sentence should have a comma between "exit" and "at". Otherwise, well, of course you can only have one statement at the end of the function! – 000 Jun 18 '13 at 19:33
  • 23
    The MISRA rule is a **terrible** rule. Especially in C++, where you can just `return` and rely on RAII. – Massa Jun 18 '13 at 19:37
  • 1
    +1 for the, surprisingly good, "duffing" article – sehe Jun 18 '13 at 19:53
  • I didn't care for the duffing article, as it presents strawman code as the only alternative to the style it's promoting. There are other ways to structure most of those samples that are (I believe) even more readable, testable, and maintainable than the ones espoused. – Adrian McCarthy Jun 18 '13 at 20:07
  • 4
    @Massa MISRA didn't use their own brains, they merely cited IEC 61508, incorrectly assuming that it was a standard written by sane people. See my comment to the question for sources. I even pointed this out to the committee during the public review of MISRA-C12 but they preferred to still avoid any rationale behind their rules. – Lundin Jun 18 '13 at 20:14
  • 2
    MISRA C:2012 has revised the rule to Advisory - and I stand by the assertion that the Rule is correct *in most cases*. However, there are situations when complying with this Rule makes things less readable/maintainable that non-complying. – Andrew Jul 24 '13 at 13:45
  • @Lundin makes an unfair comment about the MISRA Committee (note declaration in my profile) and in particular with reference to IEC61508. Whether (s)he likes it or not, 61508 is cited by EU directives and other "legal" stuff, so compliance matters, – Andrew Jul 24 '13 at 13:46
  • @Andrew I should have precised the quote came from MISRA-C:2004. I was not aware MISRA-C:2012 had been released at the time. Thanks for the information. – ouah Jul 24 '13 at 13:51
  • 1
    @Andrew Exactly which EU directive lists IEC 61508 as a normative standard? Regarding the actual _scientific source_ behind this, you go MISRA-C, which cites IEC61508, which cites (IEC 61508:7 C.2.9) one single, completely outdated source for this belief: [a 34 year old programming book](http://www.amazon.com/Structured-Design-Fundamentals-Discipline-Computer/dp/0138544719/ref=sr_1_5?ie=UTF8&qid=1375778891&sr=8-5&keywords=E.+Yourdon). Like it or not, that computer science relic is _the only rationale_ 61508 and thereby MISRA provides. So I stand by the not using their own brains comment. – Lundin Aug 06 '13 at 08:50
  • Let's start with European Machinery Directive 2006/42/EC which mandates a Hazard Analysis and Risk Assessment, which must be performed and documented by the vehicle OEM according to ISO 13849 (or another functional safety standard as required by national law). ISO 13849 refers to IEC-61508. – Andrew Aug 06 '13 at 20:55
  • @Andrew You can implement a product according to ISO 12100 and 13849-1 without using IEC 61508, it is just listed as reference in the latter. 61508 is not a harmonized standard for the machinery directive. See [EU 2013/C 99/01](http://www.machinebuilding.net/service/mmgo.php?http://eur-lex.europa.eu/LexUriServ/LexUriServ.do?uri=OJ:C:2013:099:0001:0064:EN:PDF). Anyway, despite ISO claims, 13849-1 is thoroughly unsuitable for as a guide to safe software development. Where 61508 is questionable, 13849 is laughable. Any programmer should read for example Annex J4 for a good laugh / 70s nostalgia. – Lundin Aug 08 '13 at 09:38
  • @Lundin - I don't disagree with you... you *can* yes, but that requires procurement people to understand. And I argued long and hard for there to be no mention of 26262 in MISRA C, and questioned the references to 61508... but I was in the minority, which was probably inevitable when leading proponents in 26262 are part of MISRA... – Andrew Aug 08 '13 at 10:13
  • Anyway... we're veering way off thread now... :) – Andrew Aug 08 '13 at 10:13
5

I would have written that like so because the else is redundant:

   if (a < b) {
       return 0;  // bail, since condition is met
   }
   // lots of calculations in this block
   return 1;

I don't think there's a rule of thumb, but if the function is really long and you have multiple return points, it can be hard to maintain and understand.

However, in a recursive function, for example, it's very handy to put your "base cases" as return statements at the beginning of a function.

For example, consider factorial:

int fact(int x) {
  // base cases
  if (x == 0 || x == 1)
    return 1;

  // recursive call
  return x * fact(x-1);
}

You could also write it like this:

int fact(int x) {
  int ret = 0;
  if (x == 0 || x == 1)
    ret = 1;
  else
    ret = x * fact(x-1);
  return ret;
}

I just like the first way better, but it doesn't mean either way is better than the other.

It really comes down to whatever standards you must follow and personal preference.

Joel
  • 5,618
  • 1
  • 20
  • 19
dcp
  • 54,410
  • 22
  • 144
  • 164
1

Some consider it bad practice, I'm personally ok with it as it can be cleaner-looking code. You can also do it in this way:

foo() {
   int results = 0;  // good practice to initialize
   if (a < b) {
       results = 0;  // redundant, likely optimized out
   } else {
       // lots of calculations in this block
       results = 1;
   }
   return results;
}
mark
  • 5,269
  • 2
  • 21
  • 34
0

Multiple return statements is perfectly valid in C/C++. But make sure that at least one of the return statements is always executed. The following function is incorrect,

int foo(int x)
{
  if(x>0)
  {
    return 1;
  }
  else if(x==0)
  {
    return 0;
  }
  else
  {
    // No return in this block
  }
}

such situations should be avoided in C/C++.

Deepu
  • 7,592
  • 4
  • 25
  • 47
0

Having multiple return statements in a function is perfectly acceptable. In fact, having multiple return statements in a function like the one listed above can provide performance and readability improvements. For example, you don't need the else block in the above function because you return from the function if the condition is met.

Just make sure that if your function does not have a return type of void, then you have a return statement at the end, and that all return statements return that type. For example, if your function is declared like this:

int foo ();

Then all your return statements must return integers, and you should return an integer at the end, no matter what. But if your function has a return type of void, like this:

void foo ();

Then if you reach the end of the function and have no return keyword, the function will return automatically.

creXALBO
  • 307
  • 1
  • 3
  • 13