Was ist ein besserer Weg, um durch zwei Streams zu iterieren und Anwendung auf eine

Question

Wenn jemand kann eine bessere Möglichkeit, es zu Wort, würde ich mich über eine Bearbeitung für den Titel freuen.

Ich habe eine Klasse, die eine Sammlung mit einer Kapazität darstellt.

Der Code, den ich habe, ist

public class PlayerParty implements Party { 

    public PlayerParty() { 
     this(Collections.emptyList()); 
    } 

    public PlayerParty(Collection<Pokemon> pokemon) { 
     Objects.requireNonNull(pokemon, "pokemon must not be null"); 
     if (pokemon.size() > PARTY_LIMIT) { 
      throw new IllegalArgumentException(String.format(PARTY_LIMIT_EXCEEDED, PARTY_LIMIT)); 
     } 

     createPartySlots(); 
     fillPartySlots(pokemon); 
    } 

    @Override 
    public final Iterable<Pokemon> getPokemon() { 
     return Collections.unmodifiableCollection(
       partySlots 
        .stream() 
        .filter(PartySlot::isFull) 
        .map(PartySlot::getPokemon) 
        .collect(Collectors.toList())); 
    } 

    public final Optional<PartySlot> getNextSlot() { 
     return partySlots 
        .stream() 
        .filter(slot -> !slot.isFull()) 
        .findFirst(); 
    } 

    private void createPartySlots() { 
     for (int i = 0; i < PARTY_LIMIT; i++) { 
      partySlots.add(new PartySlot()); 
     } 
    } 

    private void fillPartySlots(Iterable<Pokemon> pokemon) { 
     pokemon.forEach(p -> { 
      // Since we just added all of the slots, they're 
      // guaranteed to be present 
      // noinspection OptionalGetWithoutIsPresent 
      PartySlot slot = getNextSlot().get(); 
      slot.fill(p); 
      partySlots.add(slot); 
     }); 
    } 

    private static final String PARTY_LIMIT_EXCEEDED = "party cannot have more than %s Pokemon"; 
    private static final int PARTY_LIMIT = 6; 
    private final List<PartySlot> partySlots = new ArrayList<>(); 
} 

Verfahren in Frage um fillPartySlots dreht.

Auf der Leitung PartySlot slot = getNextSlot().get();, bekomme ich eine Warnung, die ich anrufe get ohne einen Anruf an isPresent zuerst. Dies ist verständlich, denn normalerweise würde ich das tun wollen, bevor ich versuche, einen Wert aus dem Optional herauszuholen.

Gibt es eine bessere Möglichkeit, eine Operation in einem Stream auszuführen, basierend auf dem Status eines anderen Streams? Nämlich, kann ich das ändern, so dass es isPresent verwendet (was immer in dieser Methode wahr sein sollte, weil der vorherige Aufruf im Konstruktor sie erstellt)? Kann ich etwas tun, wie

partySlots 
    .stream() 
    .filter(slot -> !slot.isFull()) // should return true for all slots 
    .forEach(slot -> { 
     slot.fill(nextAvailableOneFromMethodParameter?); 
    }); 

Answer 1

Es ist OK .get() anrufen, ohne .isPresent() prüft wird, ob der Fall des abwesenden Wertes einen Fehler darstellt. Wenn der Wert abwesend ist, wird .get() mit einem NoSuchElementException schnell ausfallen, was Sie wahrscheinlich wollen.

Möglicherweise möchten Sie dies deutlicher in Ihrem Code mit .orElseThrow(...) anstelle von .get() dokumentieren, aber das ist eine Frage der Präferenz.

Auf der anderen Seite wird Ihr vorgeschlagener Code mit .filter(slot -> !slot.isFull()) den Fall ignorieren, in dem der Slot voll ist und einen solchen Fehler schwerer zu finden macht. Darüber hinaus iteriert die alternative Implementierung über Slots und nicht über das mitgelieferte Pokemon, aber was ist, wenn es weniger pokemon als Slots gibt?

Nebenbei, haben Sie einen guten Grund, Iterable zu verwenden? Collection oder List ist wahrscheinlich eine geeignetere Wahl. Überlegen Sie, ob Sie diese Nachricht an code review senden möchten.