CODEWITHSUNDEEP
cloopsrefactoring
berbece dorel
berbece dorel

Reputation: 21

Simplify condition on loop

Have this loop statement

for (i = 0; i < 10; i++) {
    if ((i == 4) || (i == 5) || (i == 7) || (i == 9))
        continue;
    /*some code */
}

I want to make more elegant this syntax:

if ((i == 4) || (i == 5) || (i == 7) || (i == 9))

Upvotes: 2

Views: 105

Answers (6)

Robert
Robert

Reputation: 31

// comment on WHY these numbers!!!
int values[] = { 0, 1, 2, 3, 6, 8 };

for (i = 0; i < sizeof(values)/sizeof(values[0]); i++ )
{
    int value = values[i];
    ...
}

Upvotes: 0

MarcusJ
MarcusJ

Reputation: 159

Kinda surprised that no one has stated the obvious solution yet...

Anyway, What I would do in your situation is replace your loop:

if ((i == 4) || (i == 5) || (i == 7) || (i == 9))
    continue;
/*some code */

with

if (i == (4|5|7|9)) {
    continue;
/* some code */
}

Upvotes: -1

Drew
Drew

Reputation: 51

I think your code looks fine as is, but if you're looking for something more aesthetically pleasing in the 'if' conditional you could do:

for(i=0;i<10;i++){
  if(foo(i)){
        continue;
       /*some code */
  }
}

int foo(int i){
    return ((i == 4) || (i == 5) || (i == 7) || (i == 9));
}

Upvotes: 0

Sourav Ghosh
Sourav Ghosh

Reputation: 134356

I do not know the exact meaning of elegant here, but a cleaner and more maintainable approach is to use a fall-through switch-case, like

for (i = 0; i < 10; i++) {
    switch(i)
    {
        case 4:
        case 5:
        case 7:
        case 9:
              continue;
      default:
               //otherewise
    }
}

In case, later you want to modify, it is easier to understand and maintain.

EDIT:

If you're working with a coding standard that does not allow fall-through, then, the current code is just fine. At most, if required, you can create an additional function to check for the validity of i and based on the true/false (0/1) return value, you can make the decision. The additional function will have the if check.

Upvotes: 3

unwind
unwind

Reputation: 399949

You can iterate over the indices you want, a bit cumbersome but at least possible and makes it even more explicit:

const int indices[] = { 0, 1, 2, 3, 6, 8 };

for(size_t j = 0; j < sizeof indices / sizeof *indices; ++j)
{
  const int i = indices[j];
  // Rest of your loop body here, i iterates over desired values.
}

This also removes the conditional. Very hard to say anything about the performance at this point of course. Those tests weren't free either.

Upvotes: 4

Eugene Sh.
Eugene Sh.

Reputation: 18371

Well, perhaps some bit-magic?

#define MAGIC_NUMBER 0x2B0 // binary  001010110000 with bits 4,5,7,9 set

...

if ( (MAGIC_NUMBER >> i) & 0x1)
    continue;

Update: The original code is just fine. This is presented just as an alternative if really required.

Upvotes: 1

Related Questions