Replace simulated Java lists with Iterable

While cleaning old Java code I ran across this pre-Tiger pattern several times:

class Thingies {
    private final List things = new ArrayList();

    // Other useful fields

    public List getThings() {
        return Collections.unmodifiableList(things);
    }

    public void addThing(final Thing thing) {
        things.add(thing);
    }

    // Other useful methods
}

Essentially the author intended to simulate a sort of List decorated with other, useful information for a business object. Hiding the list internally is a Good Thing and helps abstraction. Using unmodifiableList is great.

But with Tiger and generics, I can do better:

class Thingies implements Iterable<Thing> {
    private final List things = new ArrayList();

    // Other useful fields

    public Iterator<Thing> iterator() {
        return things.iterator();
    }

    public void addThing(final Thing thing) {
        things.add(thing);
    }

    // Other useful methods
}

Now I can use the class with Tiger foreach syntax:

for (final Thing thing : instanceOfThingies) {
    // Do something with thing.
}

This advice is available in many places on the Internet, and is good advice. But did you notice something wrong with the example? What happens with thingies.iterator().remove()? I introduced an abstraction leak.

Let's fix the leak:

public Iterable<Thing> iterator() {
    return new Iterator<Thing>() {
        final Iterator<Thing> it = things.iterator();

        public boolean hasNext() {
            return it.hasNext();
        }

        public Thing next() {
            return it.next();
        }

        public void remove() {
            throw new UnsupportedOperationException();
        }
    };
}

Now if the caller decides to use thingies.iterator() directly, they still cannot modify the underlying list.

A neat trick

Continuing this theme, I also ran across a neat trick. The old code has a small class hierarchy which the simulated list managed. Typical usage:

class FourWheeler {
}

class Sedan extends FourWheeler {
}

class Pickup extends FourWheeler {
}

// In another class far, far away:
final Iterator sedans = fourWheelers.getList().iterator();
while (sedans.hasNext()) {
    final Sedan sedan = (Sedan) it.next();
}

// In yet another class even more far, far away:
final Iterator pickups = fourWheelers.getList().iterator();
while (pickups.hasNext()) {
    final Pickups pickup = (Pickup) it.next();
}

The thing is, the four wheelers list manager only held one type of four wheeler at a time so the caller was safe in casting as long as they were careful in updating the list. But this is very wordy. One approach is to generify the list manager:

class FourWheelers<T extends FourWheeler> {
    private final List<T> list = new ArrayList<T>();

    // Rest of class uses T instead of FourWheeler.
}

This is a great approach to the problem for new code. But there is a smaller change one can make to the class that also works. Add a new method:

public Iterable<Sedan> sedans() {
    return new Iterable<Sedan>() {
        public Iterator<Sedan> iterator() {
            final Iterator it = list.iterator();

            public boolean hasNext() {
                return it.hasNext();
            }

            @SuppressWarnings({"unchecked"})
            public Sedan next() {
                return (Sedan) it.next();
            }

            public void remove() {
                throw new UnsupportedOperationException();
            }
        }
    };
}

And likewise for Pickup. You can abstract this common pattern to a base class and simplify the methods further:

public Iterable<Sedan> sedans() {
    return new FourWheelerIterable<Sedan>(list.iterator());
}

Usage is now like this with all the nasty casting hidden away:

// Back in a far, far away class.
for (final Sedan sedan : fourWheelers.sedans()) {
    // Do something with sedan.
}

UPDATE: Jörg has a great observation in the comments: you may need more than one kind of generic Iterable<T> for a complex class.