4

At the moment, I have this:

 do{
           if (i < y){ //y is a constant

               ObsHandler.obsPartsHandler(); //increment i twice
               ObsHandler.search(); 

           }
           else { 
               break;
           }

       } while (!endline.equals(null)); // endline is changed during each loop cycle

With this loop, and the inputs I have, endline can not be null; this renders the while loop breaking condition redundant. When I try and convert this loop to a for loop, I get a misplaced construct error from Eclipse.

i.e:

for (i < y) {
       ObsHandler.obsPartsHandler(); //increment i twice
       ObsHandler.search(); }

Whilst, the while loop I have works, it seems like bad practice to me. Any suggestions? Thanks.

1
  • 99% of for loops are used to go through an array/collection. For other stuff use a while loop. Your code is ok. Commented Jul 6, 2012 at 13:39

4 Answers 4

6

If you don't need to check endLine, you can still use a while loop:

while (i < y) {    
    ObsHandler.obsPartsHandler(); //increment i twice
    ObsHandler.search(); 
}

If you also need to check endLine:

while (i < y && !endline.equals(null)) {    
    ObsHandler.obsPartsHandler(); //increment i twice
    ObsHandler.search(); 
}

Note: the difference between do / while (your initial code) and while { } is that in the first case the loop is always run at least once. With a while { }, it is not run at all if the condition is not true at the beginning. That is the same behaviour as a for loop (won't run if the condition is false before the loop).

Sign up to request clarification or add additional context in comments.

3 Comments

Thanks, that's exactly what I was looking for. I just wasn't sure if a while loop was the correct form.
This loop is indeed the correct form. But I strongly advise changing i and y to something a little more meaningful.
Normally, I'd agree but in the context of the entire project they are meaningful.
6

If you want to use a for loop, you need to have all three parts of a standard for. Since you have two conditions, they need to be both included in the condition part:

for ( ; i < y && !endline.equals(null); ) {
    ObsHandler.obsPartsHandler();
    ObsHandler.search();
}

You probably have some initialization code for i, like i=0, which could also go into the initialization section of the for loop. Otherwise, if all you have is a set of conditions to check without some array or list you are iterating, while or do is really the better fit.

5 Comments

I like the whole statement except that while or do is a better fit. While loops were designed for waiting on a logical condition. For loops were specifically made for the specific logical condition of "I want to run x number of times". The convention can be broken and either used as either, but bottom line if you know exactly how many times to execute, a for loop is superior to a while loop specifically for the reason you outlined, which is that your index variable can be init'd with the for loop, thus limiting its scope.
@corsiKa: Good point, thanks for adding your input. It does seem in this case while would be ok simply because the conditions are modified directly as a side-effect in ObsHandler.obsPartsHandler(), rather than some index or counter that is incremented throughout the loop.
Thanks, I was just seeking reassurance that this use of a while loop would not be frowned upon.
@SeanKenny: I would double-check for one-off error, because I'm not sure I've captured the meaning exactly how I've combined the conditions.
@SeanKenny What is frowned upon is using i as a variable that gets changed in multiple methods like that. Not only is it making debugging difficult for that because i is a HORRIBLE variable name for anything other than a simple for loop, but it also will confuse anyone else who tries to use for loop. If they accidentially make their for loop somewhere else in the code go for(i = 0; i <... and for get the int (which happens late in the day/night...) then it's a bug that might go undetected for quite a while.
2

A while loop seems like the natural choice for your code. I would just simplify it to:

do {
  ObsHandler.obsPartsHandler(); //increment i twice
  ObsHandler.search(); 
} while (i < y && !endline.equals(null)); // endline is changed during each loop cycle

or, if you need to check the conditions before:

while (i < y && !endline.equals(null)) {
  ObsHandler.obsPartsHandler(); //increment i twice
  ObsHandler.search();
}

Comments

2
for (boolean start=true;(start || endline != null) && i < y;){
    start=false;
    ObsHandler.obsPartsHandler(); //increment i twice
    ObsHandler.search();
}

(startendline != null) is only tested the second time.
i < y is only tested the first time.

3 Comments

What is the purpose of the boolean?
The question mentions that is not possible for endline to equal null and that the outer loop condition is not longer needed.
fair enough but I think the original poster was confused about what he was trying to achieve - what you show is equivalent to a do / while but that's not necessarily what he wanted. See the note at the bottom of my answer.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.