2つのストリームを繰り返し処理して1つの操作に適用するより良い方法は何ですか

Question

誰かがより良い言葉を語りかけることができれば、そのタイトルの編集に感謝します。

私は、容量のあるコレクションを表すクラスを持っています。

私が持っているコードは、問題の方法

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<>(); 
} 

あるfillPartySlotsを中心に展開。

PartySlot slot = getNextSlot().get();の行には、私がgetに電話していないことを示す警告が表示され、isPresentが最初にコールされます。これはわかりやすいですが、通常はOptionalの値を取得しようとする前にこれを実行したいと考えています。

別のストリームの状態に基づいて、あるストリームで操作を実行するより良い方法はありますか？つまり、これを変更してisPresentを使用するようにすることができます（これはコンストラクタの前回の呼び出しによって生成されるため、常にこのメソッドに当てはまるはずです）。私はそれがない値の場合は、バグを表す場合.isPresent()をチェックせずに.get()を呼び出すためにOKです

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

Answer 1

ような何かを行うことができます。値が存在しない場合、.get()はおそらくNoSuchElementExceptionで失敗します。

.get()の代わりに.orElseThrow(...)を使用してコード内でより明確に文書化したい場合がありますが、それは優先事項です。

一方、.filter(slot -> !slot.isFull())の提案コードでは、スロットがいっぱいになっている場合を黙って無視し、そのようなバグを見つけにくくします。また、代わりの実装では、提供されたポケモンではなく、スロットを繰り返しますが、スロットよりもポケモンが少ない場合はどうなりますか？

脇に、Iterableを使用する正当な理由はありますか？おそらく、より適切な選択肢はCollectionまたはListでしょう。これをcode reviewに投稿することを検討してください。