2

I have a getter that returns an unmodifiable list, as such:

public List<Product> getProductList() {
    if (productList == null)
        return new ArrayList<>();
    return Collections.unmodifiableList(productList);
}

I am calling this getter as such:

List<Product> productList = new ArrayList<>(report.getProductList());

Then I pass this list to another method that modifies the list as such:

for (Product product : productList) {
    product.addToAdvisoryList(advisory);
}

where addToAdvisoryList(Advisory advisory) is:

public void addToAdvisoryList(Advisory advisory) {
    if (advisoryList == null) {
        setAdvisoryList(Collections.singletonList(advisory));
    } else if (!isContainedAdvisory(advisoryList, advisory)) {
        List<Advisory> newAdvisoryList = new ArrayList<>(advisoryList);
        newAdvisoryList.add(advisory);
        setAdvisoryList(newAdvisoryList);
    }
}

After running these code, the original product list is modified. Can someone please explain what exactly happened? and what can be done to avoid modifying an unmodifiable list?

4

2 回答 2

0

In the code you provided, the original (unmodifiable) list is passed as an argument to the constructor of a java.util.ArrayList, a modifiable list.

List<Advisory> newAdvisoryList = new ArrayList<>(advisoryList);

This constructor creates a new list with all the items of the supplied Collection in the order returned by its iterator (see the documentation).

The same thing happens with the list of products:

List<Product> productList = new ArrayList<>(report.getProductList());

This makes it redundant to return unmodifiable lists in the first place.

In other words, you're not modifying unmodifiable lists. You are creating new lists that are modifiable, with the elements from unmodifiable (immutable) lists, and then modifying the (mutable) lists.

Also see here for immutable lists, from the Java documentation. You will get an UnsupportedOperationException when trying to modify an immutable list.

EDIT

To respond to your question about getters:

"Isn't the purpose of a getter to return a read-only copy of the object?"

A getter is used to provide access to a private or protected class member from the outside.

If you have a class like (in your example) Report

public class Report {

    private List<Product> products;

    public void setProducts(List<Product> products) {
        this.products = products;
    }

    public List<Product> getProducts() {
        return products;
    }

}

The getProducts makes the list accessible from outside the class (so you can add or remove elements if it's mutable). This kind of thing is usually done with Java beans, though its use is debated (consider replacing getters and setters with methods that don't unnecessarily expose state information). In any case, see here for more information on getters and setters according to Oracle.

于 2018-07-09T19:57:27.130 回答
0

Your lists contain mutable Product objects.

You don't modify any of the lists. The lists hold references to the same mutable objects, and you are making changes to these objects. It's basically the same old problem as

To avoid this, you can make getProductList return a list of copies. Alternatively you can make the Product class immutable, which will force you to rethink your approach.

public List<Product> getProductList() {
    List<Product> copies = new ArrayList<>();
    for (Product product: productList)
        copies.add(new Product(product)); // add this constructor

    return copies;
}
于 2018-07-09T20:22:02.563 回答