Лучше для цикла без перерыва;

Question

Мне было интересно, есть ли более эффективная альтернатива, чем использование этой реализации. Я пытаюсь улучшить свои способности, и следующее действительно работает. Просто я продолжаю слышать о Брейке; как легкий выход и создает потенциальный код спагетти, который не произошел здесь, но все же.

public void getWoodSoldRecently(){ 
    Calendar cal = Calendar.getInstance(); 
    cal.add(Calendar.WEEK_OF_YEAR, -2); 
    for(Tree t : theTrees){ 
     if(t.getSimpleDateSold().getTime().after(cal.getTime()) && t.getHasBeenSold()==true){ 
      treesSold.add(t); 
      System.out.println(t.getTreeId() + " " + t.getTreeType()); 
     } 
     else{ 
      System.out.println("Nothing sold in the last 2 weeks"); 
      break; //Stop the above 
     } 
    } 
} 

Без перерыва «Ничего не продается за последние 2 недели» выводит сумму, хранящуюся в массиве.

Answer 1

Поскольку вы не делаете ничего после перерыва, если оно происходит. Вы могли бы заменить его возвратом.

public void getWoodSoldRecently(){ 
Calendar cal = Calendar.getInstance(); 
cal.add(Calendar.WEEK_OF_YEAR, -2); 
for(Tree t : theTrees){ 
    if(t.getSimpleDateSold().getTime().after(cal.getTime()) && t.getHasBeenSold()==true){ 
     treesSold.add(t); 
     System.out.println(t.getTreeId() + " " + t.getTreeType()); 
    } 
    else{ 
     System.out.println("Nothing sold in the last 2 weeks"); 
     return; //Exit function 
    } 
} 

}

я лично не имею broblem с перерывами, однако вернуть силы инкапсулировать код в более функции, которым является всегда хорошо.

Answer 2

Перерыв не плох по определению - его можно использовать неправильно, но способ, которым вы его используете, разрешен. Вы можете рассмотреть возможность замены разрыва с возвратом.

Answer 3

Я не использую break в вашем примере. Все еще размещайте одно возможное обходное решение (избегайте разрыва):

boolean bContinue = true; 
    int iSize = theTrees.size(); 
    for(int indx=0; indx < iSize && bContinue; indx++){ 
    Tree t = theTrees.get(indx); 
    if(t.getSimpleDateSold().getTime().after(cal.getTime()) 
     && t.getHasBeenSold()==true){ 
     treesSold.add(t); 
     System.out.println(t.getTreeId() + " " + t.getTreeType()); 
    } 
    else{ 
     System.out.println("Nothing sold in the last 2 weeks"); 
     bContinue = false;//This will stop the loop 
    } 
} 

Answer 4

Я подозреваю, что вы вообще не хотите ломать здесь. Конечно, вы хотите просто идти с остальными деревьями - только потому, что вы найдете одно дерево, которое не соответствует вашему состоянию (например, оно не продано), действительно ли это означает, что не может быть никаких подходящих позже в вашей коллекции?

Ваш метод явно странный, так как это метод get, который не возвращает ничего, но добавляет значения в существующую коллекцию. Это первое, что я бы изменил.

Answer 5

Проблема в том, что вы используете цикл foreach, который не позволяет вам использовать условные параметры. Если преобразовать его в цикл вы будете лучше:

boolean loopContinue = true; 

for(int i = 0; i < theTrees.size() && loopContinue; i++) { 
     Tree t = theTrees.get(i); 
     if(t.getSimpleDateSold().getTime().after(cal.getTime()) && t.getHasBeenSold()==true){ 
      treesSold.add(t); 
      System.out.println(t.getTreeId() + " " + t.getTreeType()); 
     } 
     else{ 
      System.out.println("Nothing sold in the last 2 weeks"); 
      loopContinue = false; //Stop the above 
     } 
    } 
} 

EDIT & Разъяснение: я говорю «лучше» означает, что вы будете иметь больше контроля над вашей петли. Как говорили другие, использование break не является изначально плохим.

Answer 6

Работа с итератора:

public void getWoodSoldRecently(){ 
Calendar cal = Calendar.getInstance(); 
cal.add(Calendar.WEEK_OF_YEAR, -2); 
Iterator itr = theTrees.iterator(); 
boolen b = true; 
while (itr.hasNext() && b == true) { 
     if(t.getSimpleDateSold().getTime().after(cal.getTime()) && t.getHasBeenSold()==true){ 
      treesSold.add(t); 
      System.out.println(t.getTreeId() + " " + t.getTreeType()); 
     } 
     else{ 
      System.out.println("Nothing sold in the last 2 weeks"); 
      b = false; 
     } 
    } 
}