Java Surprise: Setters/Getters and Collections

Inspired by the Java Killers series, I wanted to post a problem I stumbled across some years ago:

Imagine a bean containing a field.

class Car {
private final List tires = new ArrayList();

public void setTires( List tires ) {
this.tires.clear();
this.tires.addAll( tires );
}

public void addTire( Tire tires ) {
this.tires.add( tires );
}

public List getTires() {
return Collections.unmodifiableList( tires );
}
}

What is the output of that code?

Car car = new Car();
car.addTire( new Tire() );
car.addTire( new Tire() );

List carTires = car.getTires();
System.out.println( "before: " + carTires.size() );
car.setTires( carTires );
System.out.println( "after1: " + car.getTires().size() );
System.out.println( "after2: " + carTires.size() );

The first part is easy:

before: 2

But the rest?

before: 2
after1: 0
after2: 0

We have managed to empty the collection accidentally – even if it is unmodifiable.
The problem lies in the setter and how Collections.unmodifiableList() is implemented:

Collections.unmodifiableList returns a list that is backed by the original list. Every change to the original collections is reflected.
Therefore the “clear” call empties also the unmodifiable list.

public void setTires( List tires ) {
this.tires.clear(); //the list is cleared! But since the method parameter is a view of this.tires, it is emptied, too.
this.tires.addAll( tires ); //since tires is empty now, addAll doesn't do anything...
}

So be careful with collections. They might change a really surprising moments…

Tags: , , ,

This entry was posted on Thursday, August 5th, 2010 at 11:27and is filed under Java. You can follow any responses to this entry through the RSS 2.0 feed. You can leave a response, or trackback from your own site.

7 Responses to “Java Surprise: Setters/Getters and Collections”

  • Csaba Palfi Says:
    August 6th, 2010 at 06:25

    That’s why you should use the Google Guava libraries and ImmutableList instead of Collections.unmodifiableList()

  • patrick Says:
    August 6th, 2010 at 09:29

    well that’s interesting to know but setTires() is misleading imo, i’d rather name it something like addAndReplaceTires() or something along those lines… i think a proper setTire should have a method body like this: { this.tires = tires }

    however, in that scenario we’d set an unmodifiable list and the next call to addTire would end in an exception…

  • Marco Says:
    August 6th, 2010 at 11:46

    Isn’t this basically a nontrivial case of the following (given that Java is pass-by-reference and not pass-by-value)?

    StringBuffer var = new StringBuffer(“blah”);
    StringBuffer mof = var;
    mof.append(” Blah!”);
    // var = “blah Blah!”

  • johannes Says:
    August 6th, 2010 at 13:02

    Yes. You are right.

    Java really needed true immutable lists. Therefore I really like Guava…

    Unfortunately there is no “appendElement” method that creates a new immutable list by appending just one element (or another list)…

  • johannes Says:
    August 6th, 2010 at 13:04

    There is a problem – the setter does not work as expected under all circumstances.

    Yes, I could rename the method. But often I want to provide a setter. Just exposing my bugs using the method name doesn’t solve the problem…

    So I think there should be used a completely different implementation of the setter…

  • johannes Says:
    August 6th, 2010 at 13:08

    Yes – it is the pass-by-reference thing that bites you. In combination with the fact that Collections.unmodifiableList does *not* return a never changing collection but just a view of a collection (that might change).

    But where should you know from, when you get that list as return value?
    So as long as really immutable lists are used, one is forced to always copy the collections to be safe…

  • Lyle Says:
    January 19th, 2011 at 18:38

    Java is most certainly pass-by-value.

Leave a Reply