Please suggest alternatives to this if-else construct

[kalf]

The project is in C, so I apologise to all the C++ purists on the board, who might want to suggest more elegant C++ solutions. I believe I'm not completely out of line posting here though, since C is a "subset" of C++, and the real problem is less about language features and more about logic, which exists in abundance on this forum (I'm a champion at sucking up ;-)).

I've got some code that has fallen foul of a coding standards review. The alleged reason being that I've used an assignment in an if statement.

Personally, I think they've missed the point, but I still have to change the code. I'm looking for suggestions on how to implement a clean alternative.

Other rules that restrict the solution are that I'm only allowed one point of exit from the function, and I can't use break or goto (not that I'd want to in this situation).

The code in question looks like:

int status = -1;
...
if ((status = functionReturningInt()) != 0)
{
   // log the errors
   // clean-up all work done so far
   ...
}
else if ((status = anotherFunctionReturningInt()) != 0)
{
   // log the errors
   // clean-up all work done so far
   ...
}
else if ((status = yetAnotherFunctionReturningInt()) != 0
{
   // log the errors
   // clean-up all work done so far
   ...
}
...
// many more if-else
else
{
   // We got here without any errors
   // log successful status, etc.
}

// log errors,
// clean up everything

return status;

You see my problem, I don't want to continue if any of the function calls fail, so I use the if-else to make sure. Unfortunately, it's quite a long chain, and if I try and nest the if-else blocks, I break another rule about the level of nesting permitted. I imagine this problem must pop up a lot, so I'm convinced there's a simple and obvious solution I'm missing.

The cursory review assumed (I think) that I was doing something like

if (x = function())
{
   ...
}

Which, I agree is dangerous, but is not what I did.

All solutions I've thought of have appeared too messy or complicated for such a simple task. All suggestions are welcome, many thanks.

[aleonard]

Hi, I'm not sure I understood completely, but here one suggestion. You could have a return statement in your if statements. This way you leave th function directly.

[kalf]

Thanks for your answer aleonard. It makes sense, but the coding standard restricts me to a single return statement.

[Lowest0ne]

Functions, functions, functions.

It's tough to say how flexible you can make this, is cleanup always the same, are the function definitions always the same? Are the things that need cleaning global?

As a start, consider this:

#include <stdio.h>
#include <time.h>

int random( void ) { return rand() % 10; }

int func1( void ) { printf("Func1: "); return random(); }
int func2( void ) { printf("Func2: "); return random(); }
int func3( void ) { printf("Func3: "); return random(); }
int func4( void ) { printf("Func4: "); return random(); }

typedef int (*func)( void );

int main( void )
{
  srand( time(0) );

  func functions[] = { func1, func2, func3 };
  const size_t length = sizeof( functions ) / sizeof( functions[0] );

  size_t idx;
  for ( idx = 0; idx < length; ++idx )
  {
    int result = functions[idx]();
    printf( "%d\n", result );
  }

  return 0;
}

For this setup, if you wanted to start calling func4, all you have to do is tack it onto the array at line 17.


It all depends upon your setup now. If everything is global, it's simple:

int func1( void )
{
  int status = functionReturningInt( parameters );
  if ( status != 0 )
  {
    // blah
  }
  return status;
}

int func2( void )
{
  int status = anotherfunctionReturningInt( other, parameters );
  if ( status != 0 )
  {
    // blah
  }
  return status;
}

// now something like
size_t idx = 0;
while( idx < length && funct[idx]() == 0 )
{
  ++idx;
}

if ( idx == length ) // everything is okay 

[Cubbi]

incidentally, assign-and-compare in the if condition is pretty common C, the classic K&R read loop was

int c;
while((c=getchar())!=EOF) {
// do stuff with c
}

strange how it is disallowed in whatever rules this is expected to conform.

[booradley60]

Review presenters and code reviewers should be given some freedom in the software process directives to waive certain restrictions in sensible cases. I think in adhering to the letter of the law you're losing some of the spirit of the law. Maybe I'm passive-aggressive, but I'd write the code out in the standard-conforming way and show the reviewers how awful the code looks. You should be able to take this up with a chief engineer and get around the rule, maybe appending a comment to each line:
//intentional assignment to satisfy the spirit of the law, which is to promote code simplicity, clarity, and maintainability.

[kalf]

Thanks very much guys.

@LowestOne, I think I'll use your concept.

@Cubbi, I totally agree with you. I didn't expect this to fall foul of the reviewer.

I think my reviewer didn't really look at the code, he just saw the single "=" and knee-jerk rejected it. He gave a "just do it my way" response when I emailed him pointing out the difference. Maybe he was just having a bad day.

Thanks again.

[booradley60]

just do it my way

Sounds like bullshit. Call him on it.