Reducing Code Duplication with For loop in java

user_15768256 0
Viewed 111
    for (Aircraft aircraft : landingQueue) {
        if (aircraft.hasEmergency()) {
            return aircraft;
        }
    }

    for (Aircraft aircraft : landingQueue) {
        if (aircraft.getFuelPercentRemaining() <= 20) {
            return aircraft;
        }
    }

    for (Aircraft aircraft : landingQueue) {
        if (aircraft instanceof PassengerAircraft) {
            return aircraft;
        }
    }
    return landingQueue.get(0);

So my program runs through a list of aircraft in a queue and will return one based on the importance of a task. For instance, it will first check the landingQueue ArrayList to see if any aircraft are in an emergency and if it can't find one it then checks for aircraft with fuel less than 20 and so on. Is there a simple way to reduce the duplication of the for loop and if statement? Much appreciated

duplicates for-loop java
5 Answers

Another approach could be to sort your list according to your priority criteria

public Aircraft returnOneBasedOnImportance(){
    Comparator<Aircraft> byEmer = Comparator.comparing(a -> !a.hasEmergency());
    Comparator<Aircraft> byFuel = Comparator.comparing(a -> !(a.getFuelPercentRemaining() <= 20));
    Comparator<Aircraft> byType = Comparator.comparing(a -> !(a instanceof PassengerAircraft));

    return landingQueue.stream()
                       .sorted(byEmer.thenComparing(byFuel).thenComparing(byType))
                       .findFirst().get();
}
user_51769920

The improvement is not as much about code duplication, more important it is to limit the number of times you iterate through that list.

Aircraft firstEmergencyAircraft = null;
Aircraft firstLowFuelAircraft = null;
Aircraft firstPassengerAircraft = null;

for (Aircraft aircraft : landingQueue) {
        if (firstEmergencyAircraft==null && aircraft.hasEmergency()) {
            firstEmergencyAircraft = aircraft;
        }
        if (firstLowFuelAircraft==null && aircraft.getFuelPercentRemaining() <= 20) {
            firstLowFuelAircraft = aircraft;
        }
        if (firstPassengerAircraft==null && aircraft instanceof PassengerAircraft) {
            firstPassengerAircraft = aircraft;
        }
    }

if(firstEmergencyAircraft!=null){
    return firstEmergencyAircraft;
}
if(firstLowFuelAircraft!=null){
    return firstLowFuelAircraft;
}
if(firstPassengerAircraft!=null){
    return firstPassengerAircraft;
}
return landingQueue.get(0);
user_27435850

I'd make landingQueue some sort of SortedSet (i.e. TreeSet) with a Comparator that sorts by landing priority (i.e. sort criteria are emergency, low fuel level, is PassangerAircraft). Then you can always just pick the first (or last) entry from the list.

user_88674660

One option would be to create a stream of predicates first, then for each predicate loop through the list and try to find an aircraft that matches. If no match is found, return the first element.

Stream<Predicate<Aircraft>> predicates = Stream.of(
  (aircraft) -> aircraft.hasEmergency(),
  (aircraft) -> aircraft.getFuelPercentRemaining() <= 20,
  (aircraft) -> aircraft instanceof PassengerAircraft
);

return predicates
  .flatMap((predicate) -> landingQueue.stream().filter(predicate))
  .findFirst()
  .orElse(landingQueue.get(0));
user_39825620

Why not just do the tests in one loop?

for (Aircraft aircraft : landingQueue) {
    if (aircraft.hasEmergency() ||
        aircraft.getFuelPercentRemaining() <= 20 ||
        aircraft instanceof PassengerAircraft) {
        return aircraft;
    }
}

return landingQueue.get(0);
user_4047970
Related