28

Suppose I have this:

public class Unit<MobileSuit, Pilot> {

    ...

    List<MobileSuit> mobileSuits;
    List<Pilot> pilots;

    ...
}

And I would like to iterate through the pair of each in the simplest way outside of that class. How should I go about doing that? I thought about doing this:

public class Unit<MobileSuit, Pilot> {

    ...
    Iterator<MobileSuit> iteratinMechas;
    Iterator<Pilot> iteratinPeople;

    class IteratorCustom<MobileSuit, Pilot> implements Iterator {

        public boolean hasNext() {
            return iteratinMechas.hasNext() && iteratinPeople.hasNext();
        }

        public void remove() {
            iteratinMechas.remove();
            iteratinPeople.remove();
        }

        public Object next() {
            // /!\
        }

    }

    public Iterator iterator() {
        return new IteratorCustom<MobileSuit, Pilot>(mobileSuits, pilots);
    }
}

Something along those lines.

Anyway, the problem is that I can't really return just a single object from next(), and I also can't have a Iterator take more than one type. So, any thoughts?

Also, I can't make a new class to combine MobileSuit and Pilot. I need to keep them separate, even though I'm iterating through both at a time. The reason is that there might be Mobile Suits that have no pilots, and I'm not sure how to fix that by keeping them at the same class. This class needs to be processed in other places, so I'd have to unify a interface around that and a lot of other stuff. Basically, assume MobileSuit and Pilot need to be separated.

dimo414
  • 47,227
  • 18
  • 148
  • 244
Setsuna F. Seiei
  • 387
  • 1
  • 3
  • 8
  • 1
    If there are going to be mobilesuits that don't have pilots, I'm assuming you have more mobile suits than pilots. That may make your check for hasNext() problematic, since it will only return true if there are more items in both lists. I think it would help get your question answered if you could provide a bit more info on the criteria for how the two can be combined. –  Jun 29 '10 at 05:05
  • @Rob Cooney Yeah, now that you mention it, that hasNext() would be at the very least wrong. – Setsuna F. Seiei Jun 29 '10 at 05:08
  • 1
    OMG +1 for the Gundam-ish question. – polygenelubricants Jun 29 '10 at 07:39
  • 1
    Very interesting question. I hadn't heard of a zipper before http://stackoverflow.com/questions/1115563/what-is-zip-functional-programming so then I wondered "Why doesn't Guava support it?" Apparently it does internally. There has been some discussion about supporting it externally - see http://code.google.com/p/guava-libraries/issues/detail?id=35 There is a related question here http://stackoverflow.com/questions/5278040/java-join-collections-using-functor/ – Mark Butler Feb 08 '13 at 05:48
  • For iterating through parallel collections generally, see [How to most elegantly iterate through parallel collections?](http://stackoverflow.com/questions/1365793/how-to-most-elegantly-iterate-through-parallel-collections) though here it's different because you want to iterate _outside_ the class. – Nils von Barth Jun 05 '15 at 05:01
  • Note that if your `Unit` class provides an `iterator()` method, you generally want to add `implements Iterable<...>` to the class signature. – dimo414 Feb 24 '17 at 17:01

11 Answers11

12

Anyway, the problem is that I can't really return just a single object from next(), and I also can't have a Iterator take more than one type. So, any thoughts?

Obviously you are going to need a light-weight "pair" class. This is roughly analogous to the Map.Entry inner class.

Here's a rough cut at a generic solution:

public class ParallelIterator <T1, T2> implements Iterator<Pair<T1, T2>> {

    public class Pair<TT1, TT2> {
        private final TT1 v1;
        private final TT2 v2;
        private Pair(TT1 v1, TT2 v2) { this.v1 = v1; this.v2 = v2; }
        ...
    }

    private final Iterator<T1> it1;
    private final Iterator<T2> it2;

    public ParallelIterator(Iterator<T1> it1, Iterator<T2> it2) { 
        this.it1 = it1; this.it2 = it2;
    }

    public boolean hasNext() { return it1.hasNext() && it2.hasNext(); }

    public Pair<T1, T2> next() {
        return new Pair<T1, T2>(it1.next(), it2.next());
    }

    ...

}

Note: this doesn't explicitly deal with cases where the lists have different lengths. What will happen is that extra elements at the end of the longer list will be silently ignored.

Sarah Vessels
  • 30,930
  • 33
  • 155
  • 222
Stephen C
  • 698,415
  • 94
  • 811
  • 1,216
5

This is copied+edited from Stephen C's answer. Feel free to use:

public class Pair<T1, T2> {
    private final T1 v1;
    private final T2 v2;
    Pair(T1 v1, T2 v2) {
        this.v1 = v1;
        this.v2 = v2;
    }
    public T1 first(){
        return v1;
    }
    public T2 second(){
        return v2;
    }
}

public class ParallelIterator <T1, T2> implements Iterator<Pair<T1, T2>> {

    private final Iterator<T1> it1;
    private final Iterator<T2> it2;

    public ParallelIterator(Iterator<T1> it1, Iterator<T2> it2) { 
        this.it1 = it1; this.it2 = it2;
    }

    @Override
    public boolean hasNext() { return it1.hasNext() && it2.hasNext(); }

    @Override
    public Pair<T1, T2> next() {
        return new Pair<T1, T2>(it1.next(), it2.next());
    }

    @Override
    public void remove(){
        it1.remove();
        it2.remove();
    }
}

public class IterablePair <T1, T2> implements Iterable<Pair<T1,T2>> {
    private final List<T1> first;
    private final List<T2> second;

    public IterablePair(List<T1> first, List<T2> second) { 
        this.first = first;
        this.second = second;
    }

    @Override
    public Iterator<Pair<T1, T2>> iterator(){
        return new ParallelIterator<T1,T2>( first.iterator(), second.iterator() );
    }
}

void someFunction(){
    IterablePair<X,Y> listPair = new IterablePair<X,Y>( x, y );
    for( Pair<X,Y> pair : listPair ){
        X x = pair.first();
        ...
    }
}

This stops as soon as either list is out of elements, so you might want to check lists have equal size before creating an IterablePair.

dhardy
  • 11,175
  • 7
  • 38
  • 46
4

Also, I can't make a new class to combine MobileSuit and Pilot.

That doesn't sound correct. It sounds like you can't replace MobileSuit and Pilot by a single class, but I don't see any reason why you can't have a single class that combines them - i.e. one which just has a getPilot() method and a getMobileSuit() method. You could use a generic Pair class for the same purpose, but a custom class would be easier to use.

On the other hand, if you want to do this sort of "zipping" operation in multiple places, it might be one solution. Alternatively, you could write a generic interface to represent the act of combining the two distinct items - which could return a SuitedPilot or whatever your combination class is.

Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194
  • Even better, though the Pair class could be strange in the overall Object Model it certainly makes a good utility class for this problem. – Newtopian Jun 29 '10 at 05:02
2

The reason is that there might be Mobile Suits that have no pilots, and I'm not sure how to fix that by keeping them at the same class.

You can use null values, right? Which is the correct way of doing it - have each suit keep track of its pilot. If it has no pilot, then indicate that with a null value there.

But, if you're dead set on not doing that for some reason...

public class SuitAndPilot
{
    public MobileSuit suit;
    public Pilot pilot;

    public SuitAndPilot(Suit s, Pilot p) {
           suit = s;
           pilot = p;
    }
}
Anon.
  • 58,739
  • 8
  • 81
  • 86
1
for(int i=0; i < mobileSuits.size(); i++) {
  MobileSuit suit = mobileSuits.get(i);
  Pilot pilot = pilots.get(i);
  ...
}
dimo414
  • 47,227
  • 18
  • 148
  • 244
  • commenting on your code snippet would make your answer a lot clearer – Pankrates Mar 29 '13 at 15:40
  • That's the most straightforward solution for someone who's used to C-style index loops, I think, but it's not so elegant nor efficient, I think. – Daniel Hershcovich Apr 08 '14 at 08:17
  • You don't check that the two lists are the same size, meaning that if the `mobileSuits` list is longer you'll get an `IndexOutOfBoundsException` and if `pilots` is longer you'll silently drop some `Pilot` objects. – dimo414 Feb 24 '17 at 17:34
  • final Iterator pilotIterator = pilots.iterator(); mobileSuits.forEach(m -> { Pilot pilot = pilotIterator.next()); }); – Nestor Milyaev May 16 '19 at 14:27
1

Why not have a class MannedMobileSuit as a subclass of MobileSuit that contains an instance of a pilot ? That would solve your problem by having a getPilot method.

Usually when you get such problems (needing to return two instances) it is because your Object model is not appropriate and should be changed. Keep your options open

Newtopian
  • 7,543
  • 4
  • 48
  • 71
1

Came across this page trying to solve this issue, and turns out that there's a library out there that's already solved it using Java 8 streams (check out the Zip function).

You can convert a list to a stream just by calling list.stream()

https://github.com/poetix/protonpack

Stream<String> streamA = Stream.of("A", "B", "C");
Stream<String> streamB  = Stream.of("Apple", "Banana", "Carrot", "Doughnut");
List<String> zipped = StreamUtils.zip(streamA,
                                      streamB,
                                      (a, b) -> a + " is for " + b)
                                 .collect(Collectors.toList());

assertThat(zipped,
           contains("A is for Apple", "B is for Banana", "C is for Carrot"));
burythehammer
  • 382
  • 2
  • 6
1

Basically, assume MobileSuit and Pilot need to be separated.

That's fine, but here you're trying to treat them as a unit, so structure your code that way. The suggestions above use a Pair class or Map.Entry, but it's much better to provide a clearly-named object that represents a MobileSuit with a Pilot, e.g.:

public class OccupiedSuit {
  private final MobileSuit suit;
  private final Pilot pilot;

  public OccupiedSuit(MobileSuit suit, Pilot pilot) {
    this.suit = checkNotNull(suit);
    this.pilot = checkNotNull(pilot);
  }

  // getters, equals, hashCode, toString
  // or just use @AutoValue: https://github.com/google/auto/tree/master/value
}

Then, rather than constructing a custom Iterator/Iterable, just write a helper function that zips up the two lists. For example:

public static List<OccupiedSuit> assignPilots(
    Iterable<MobileSuit> suits, Iterable<Pilot> pilots) {
  Iterator<MobileSuit> suitsIter = suits.iterator();
  Iterator<Pilot> pilotsIter = pilots.iterator();
  ImmutableList.Builder<OccupiedSuit> builder = ImmutableList.builder();

  while (suitsIter.hasNext() && pilotsIter.hasNext()) {
    builder.add(new OccupiedSuit(suitsIter.next(), pilotsIter.next()));
  }
  // Most of the existing solutions fail to enforce that the lists are the same
  // size. That is a *classic* source of bugs. Always enforce your invariants!
  checkArgument(!suitsIter.hasNext(),
      "Unexpected extra suits: %s", ImmutableList.copyOf(suitsIter));
  checkArgument(!pilotsIter.hasNext(),
      "Unexpected extra pilots: %s", ImmutableList.copyOf(pilotsIter));
  return builder.build();
}

Now you don't need to maintain a complex custom Iterator implementation - just rely on one that already exists!


We can also generalize assignPilots() into a generic utility that works for any two inputs, like so:

public static <L,R,M> List<M> zipLists(
    BiFunction<L,R,M> factory, Iterable<L> left, Iterable<R> right) {
  Iterator<L> lIter = left.iterator();
  Iterator<R> rIter = right.iterator();
  ImmutableList.Builder<M> builder = ImmutableList.builder();

  while (lIter.hasNext() && rIter.hasNext()) {
    builder.add(factory.apply(lIter.next(), rIter.next()));
  }

  checkArgument(!lIter.hasNext(),
      "Unexpected extra left elements: %s", ImmutableList.copyOf(lIter));
  checkArgument(!rIter.hasNext(),
      "Unexpected extra right elements: %s", ImmutableList.copyOf(rIter));
  return builder.build();
}

Which you'd then invoke like so:

List<OccupiedSuit> occupiedSuits = zipLists(OccupiedSuit::new, suits, pilots);

Example code uses Guava's Preconditions and ImmutableList - if you don't use Guava it's easy enough to inline and swap to ArrayList, but just use Guava :)

Community
  • 1
  • 1
dimo414
  • 47,227
  • 18
  • 148
  • 244
0

You could just use a Map<MobileSuit, Pilot>, where a null value mapped to a MobileSuit indicates no pilot. The Iterator could just be an Iterator<Map.Entry<MobileSuit, Pilot>> retrieved by map.entrySet().iterator().

ColinD
  • 108,630
  • 30
  • 201
  • 202
  • How do you fill the Map? Do you mean replace the two lists with a Map? You could use map.keySet() and map.values() if the raw lists are needed, but then we lose ordering. – idbrii Jul 01 '10 at 17:31
  • Yes, I meant to replace the two lists with a Map. It seems clear that what's needed here is associations between MobileSuits and Pilots and that's exactly what a Map is for. Now, if there could be Pilots without a MobileSuit as well as the other way around, it might make sense to maintain the lists as well (sets would probably better) while still using a map for associating the two. – ColinD Jul 01 '10 at 18:06
  • +1 there's no reason to store these objects in separate collections if they're in practice tightly-coupled. A `Map` does change the semantics slightly (no duplicates allowed) but in general it's a much better idea. – dimo414 Feb 24 '17 at 16:56
0

Improving on the answer by user2224844, here is a simple version that will try no to run into an exception:

final Iterator<String> pilotIterator = pilots.iterator();
            mobileSuits.forEach(m -> {
                    Pilot p = pilotIterator.hasNext()? pilotIterator.next():nullOrWahtever;
<Now do your work with m and p variables>
    ...
    });
Nestor Milyaev
  • 5,845
  • 2
  • 35
  • 51
-3

Isn't that enough ?

for(MobileSuit ms : MobileSuits) {
    for(Pilot p : pilots){
        //TODO
    }
}
Aymen
  • 558
  • 6
  • 13
  • 4
    That's O(n²) instead of O(n). – Setsuna F. Seiei Jun 29 '10 at 05:04
  • This doesn't answer the question. There's an implicit relationship between the `MobileSuit` at index *i* and the `Pilot` at index *i*, and the goal here is to provide an `Iterator` that exposes that relationship. This code instead associates each `MobileSuit` with *every* `Pilot`. – dimo414 Feb 24 '17 at 17:29