Thread: Breaking a for loops when result reached

Code:

public int CurrentIndex()
     {
     int retval = -1;			
     for(int n=0;n<Images.Count && retval == -1;n++)
          {					
          if(Images[n].PosX == 0 & Images[n].PosY == 0)
               retval = Images.IndexOf(Images[n]);
          }
     return retval;
     }

edit: the reason I changed it this way is that I HATE things that break the proper flow of a function. Meaning, there should be a begin, end and a set of paths to get there but NEVER jumps. so, what that means is no gotos, breaks ("case" being an exception), returns, etc... in the middle of functions.

When a function calls return, the function quits, and thus the loop and whatever code that was in the function has all been terminated. Return means to quit the function immediately and return that value. Therefore, you do not need a break command to stop the loop. So, basically, modify your code like so:

Code:

public int CurrentIndex()
{			
                for(int n=0;n<Images.Count;n++)
	{					
	               if(Images[n].PosX == 0 & Images[n].PosY == 0)
		{
			return Images.IndexOf(Images[n]);
			//break; - NOT NEEDED
		}
		// else - DO NOT NEED AN ELSE
                                                // return -1; - WRONG PLACE
	}
	return -1; // RIGHT PLACE
}

Also, the return -1 command was in the wrong place - after your first test to see if the current object is the correct one, you either return it or -1, without ever checking any other objects. So, remove the else part of the if statement. Let the loop continue through, and if an object is found, then return quits the whole function immediately. If no object is found, the loop goes through completely, and continues on with whatever statements that follow it. So, at this point, you can return -1 to indicate nothing was found.

(EDIT: I agree with what FillYourBrain is saying about improper breaking of loops/functions. But if they are simple enough, there should not be any problem using a return in the middle of one for loop.)

no, your right retval == -1 is what you want. I'll change it above.

I like this code best, with the one modification (although it should work either way):

Originally posted by JasonD

Code:

public int CurrentIndex()
{			
	for(int n=0;n<Images.Count;n++)
	{					
		if(Images[n].PosX == 0 && Images[n].PosY == 0)
		{
			return Images.IndexOf(Images[n]);
		}
	}
	return -1;
}

I don't agree with FYB's dislike of breaking or returning in mid-loop. I would agree if we were talking about x86 assembly language; in that, you want to write the cleanup and return code only once for each function. But this is not x86 assembler, and every necessary cleanup before return should, if you program correctly, be automatic.

I think adding flag variables and returning in exactly one point often serves to obfuscate code, and it often leads to deeper if-else nesting. On the other hand, abuse of breaking, returning in a loop, etc. are also bad style.

I think there is no hard and fast rule of thumb; go with whatever code offers maximum readability. I never recommed gotos, those are completely unneeded in C++ (they are NEVER the most readable solution), but commands to prematurely end loops (break), to prematurely end a function (return or throw), etc. are very useful. Like anything else, you can abuse them, but I think it's a little foolish to disavow ever using a language construct when it makes the program more readable.

In this case, I find the above code more readable than FYB's, although of course both can be understood and both accomplish the same goal. And if we were returning an object, and not an int, the above code could be more efficient because unnamed copies of objects can often be optimized away by the compiler. This code also requires two fewer compare/jump pairs as c0ompared to FYB's, which can slow down code (especially when branch prediction fails, as it will when you exit a loop). Granted, the performance difference is almost certainly so small as to be totally negligable, except when returning objects.

My rule of thumb, which is always my rule of thumb for everything, is "use it -- don't abuse it." Write in a way that makes the program most clear. Sometimes that means breaking from loops; sometimes it means not breaking from loops.