Looping best practices

Question

I have a very large loop that loops a 1000 rows. I exit the loop if magic value 1 is found. If magic value 1 is not found but magic value 2 is found then the loop needs to skip to the beginning. Right now I am using a switch, some ifs and a goto. I have read that goto is not the best way. Is there a better way to make this work?

Answer 1

To exit a loop you can use the break statement, to go onto the next record you can use the continue statement.

for(int i = 0; i < 1000; i++)
{
    if(magicValue1)
       break;
    if(magicValue2)
       continue;
}

I AM NOT CONDONING THE USE OF THE GOTO STATEMENT I AM SIMPLY POINTING OUT A POSSIBLE USE CASE

You can use goto jump statement to start/exit a loop, however I would stay away from this option unless you are using nested looping. I think the goto statement still has its uses for optimizing, exiting cleanly ect.. but in general it is best to use it quite sparingly.

for(int i = 0; i < 100; i++)
{ 
  start:

  for(int i = 0; i < 10; i++)
  {
     if(magicValue1)
       goto end;
    if(magicValue2)
       goto start;
  }
}
end : 

Answer 2

How about this:

for(int i = 0; i < 1000; i++) {
    if(values[i] == MAGIC_VALUE_1) {
        break;
    } else if(values[i] == MAGIC_VALUE_2) {
        i = 0;
    }
}

If by "skip to the beginning" you mean "skip this record and process the next one," replace i = 0 with continue.

Answer 3

A while variation with no break:

bool continue = true; int i = 0;
while (i < 1000 && continue){
    if(values[i] == MAGIC_VALUE_1) {
        continue=false;
    } else if(values[i] == MAGIC_VALUE_2) {
        i = 0;
    }
    i++;
}

Answer 4

I can't comment yet ( 1 rep point away)

but wouldn't this be better:

for (int i = 0; i < 1000; i++)
{
    if (magicValue1)
    {
       break;
    }
    else if (magicValue2)
    {
       dosomething();
       i=0;
    }
}

and I'm not sure by whats meant by "restart the search".

Answer 5

I'm taking #2 case to mean that you want to not perform (i.e. skip) the loop body in the #2 case and not that you want to reset the loop to 0. (See the code comments if I've got that backward.)

This suggestion may be controversial because of the less conventional condition in the for loop could be said to be low on the self-documenting scale, but if that doesn't bother you, a concise way of writing what I think you you want is:

        for (int i= 0; i<values.Length && values[i]!= MAGIC_1; i++)
        {
            if (values[i] == MAGIC_2)
            {
                // Don't do the loop body for this case but continue on looping
                continue;
                // If you want to reset the loop to zero instead of skip the 2 case,
                // comment-out the continue; and un-comment the line below:
                // i=0;
            }
            // Do long loop body here
        }

Answer 6

Just note that if you set the counter back to 0 if MagicValue is 2, and your code never changes the values, you are probably going to be in an infinite loop.

Answer 7

A more complex could be:

We define 2 Extension Methods.

public static class Extensions
{
   public static bool isMagic_1(this int i)
   {
         return i == 1;
   }

   public static bool isMagic_2(this int i)
   {
         return i == 2;
   }
}

Now you can do this:

  for(int i = 0; i < 1000; i++)
  {
     if(i.isMagic_1())
       break;
     if(i.isMagic_2())
       continue;
  }

hope this helps!