Adhering to a Silly Rule

Introduction

Some coding standards, including MISRA, insist that a function only return from its end. This often results in a flag being used to bypass each subsequent condition and eventually reach the return statement. Fortunately, there is an idiom—also useful in the middle of a function—that can mitigate this dross.

Using the Code

Have you ever worked on code like this?

<type> Function(...)
{
   <type> result = <default>;
   bool done = false;

   if(condition1)
   {
      result = <something>;
      done = true;
   }

   if(!done && condition2)
   {
      result = <something else>;
      done = true;
   }

   ...

   if(!done && conditionN)
   {
      result = <something completely different>;
      done = true;
   }

   return result;
}

Or even worse, code that does the above by nesting condition statements so deeply that you have to use the horizontal scroll bar to read the code way over on the right?

Some folks actually believe this improves quality so much that they mandate it. If you find it as imbecilic as I do, here's a way to improve the code while still observing the rule.

No, we're not about to use goto to reach the return statement. Although that would be an improvement, the standard probably prohibits it too.

So instead, do this:

<type> Function(...)
{
   <type> result = <default>;

   do
   {
      if(condition1)
      {
         result = <something>;
         break;
      }

      if(condition2)
      {
         result = <something else>;
         break;
      }

      ...

      if(conditionN)
      {
         result = <something completely different>;
         break;
      }
   } while(false);

   return result;
}

That's similar to the goto solution, but I doubt your standard will have anything rude to say about it. Having to use this idiom in this situation is still unfortunate, but it's the best of a bad lot.

The idiom can be very useful in middle of a function. For example, I have a C++ parser in which the following code parses a class tempate instance:

bool Parser::ParseClassInst(ClassInst* inst, size_t pos)
{
   auto name = inst->ScopedName(true);
   Debug::Progress(CRLF + Indent() + name);

   //  Initialize the parser. If an "object code" file is being produced,
   //  insert the instance name.

   Enter(IsClassInst, name, inst->GetTemplateArgs(), inst->GetCode(), true);
   lexer_.Reposition(pos);
   Context::Trace(CxxTrace::START_TEMPLATE, inst);

   //  Push the template instance as the current scope and start to parse it.
   //  The first thing that could be encountered is a base class declaration.
   do
   {
      BaseDeclPtr base;
      Context::PushScope(inst, false);
      GetBaseDecl(base);
      if(!lexer_.NextCharIs('{')) break;
      inst->AddBase(base);
      GetMemberDecls(inst);
      Context::PopScope();
      if(!lexer_.NextCharIs('}')) break;
      if(!lexer_.NextCharIs(';')) break;
      GetInlines(inst);
   }
   while(false);

   //  The parse succeeded if the lexer reached the end of the code.  If the
   //  parse failed, indicate this on the console.  If an "object code" file
   //  is being produced, indicate that parsing of the template is complete.

   auto parsed = lexer_.Eof();
   Debug::Progress((parsed ? EMPTY_STR : " **FAILED** "));
   if(!parsed) Failure(venue_);
   Context::Trace(CxxTrace::END_TEMPLATE);
   return parsed;
}

Granted, that's not a great example. A better (but more complicated) one involves resolving each identifier in a qualified name. That do {...} while(false); occurs in one case of a while-switch loop and handles an identifier that names a class template instance, so it really helps to keep the code manageable. But I hope the above  illustrates the point.

Best of luck in your code review!

History

• 1^st May, 2021: Initial version