How does returning an arraylist break encapsulation?

Question

I'm studying java and my teacher told me that this break with the principle of encapsulation:

private ArrayList<Item> inventory; 

inventory = new ArrayList<Item>();

public List<Item> getInventory() {
    return inventory;
}

and he told me that I should not return inventory directly because it breaks encapsulation, but I should return a copy instead like this (see below) so we don't want to return it directly. Whats the point of returning two lists, when we just want to change the first (by adding items)?

public List<Item> getInventory() {
    return new Arraylist<Item>(inventory);
}

However, his explanation made no sense to me, can anyone help? Thank you for your time :)

If the goal of the getInventory method is to return something that the caller cannot modify I would however rather return an actual umodifyable List instead of a shallow copied arraylist. Also while all the answers say this is supposed to "prevent the caller from modifying the list" he might actually be able to modify the Items in the List (if they have methods for that). So I personally think if your teachers intention was to not have the caller modify anything in the return value his proposed solution only does part of the job. — OH GOD SPIDERS, Jan 19 '18 at 17:09
I don't see how it breaks encapsulation either. However, it may violate immutability of the object of which the inventory list is being returned. If the inventory is not meant to be modified outside of special methods designed to do so, then returning a reference to the original (mutable) inventory is breaking that immutability contract — smac89, Jan 19 '18 at 17:14
Ok I see how it may break encapsulation. Encapsulation is meant to protect the internals of an object from direct access and to hide implementation details. However, even though you have a method for accessing the object's `inventory`, it is no different from not having the method, because once a user gets a hold of the list, they now have access to an internal component of the object; and can change the list as they see fit. — smac89, Jan 19 '18 at 17:20
The irony here is that even though everyone says you should return copy of list but in real life almost none does that and there are a lot of libraries that just return the same list in the getter. — tsolakp, Jan 19 '18 at 17:27
@tsolakp, almost always when that happens (at least in my case), we are returning a list which we don't need to keep track of, nor does the current internal state of the application/module depend on it. For example, a service class that makes a call to a database to return a list of stuff - there is never a need to make a copy of this list because...well it's a service and all it does is return stuff. If however we have an internal state which needs to be maintained, and returning this list will allow someone to change this state, then yes a copy is required. — smac89, Jan 20 '18 at 00:14
@smac89. Take a simple pojo (service layer DTO or JPA entity) which has getter that returns list. Do you always make a copy of that list before returning. — tsolakp, Jan 20 '18 at 00:23

score 6 · Answer 1 · answered Jan 19 '18 at 17:07

Suppose I'm a caller to your library and want to remove something from inventory without asking you. I could write code to get the list of inventory items, manipulate it, and you'd never know.

List<Item> stolenInventory = yourObject.getInventory();
stolenInventory.remove(0);

Or if I wanted to add my own items without going through the proper process:

List<Item> theInventory = yourObject.getInventory();
theInventory.add(myIllegalProduct);

YOUR copy of inventory and MY copy of inventory are the same thing because they refer to the same object. By making a defensive copy, you are letting me know the inventory but I can't manipulate it in a way that impacts you, the authority on inventory items.

score 2 · Answer 2 · answered Jan 19 '18 at 17:12

The reason why this breaks encapsulation is because returning inventory directly allows any developer to directly modify inventory. The only why you should be modifying inventory is through a setter not a getter.

private ArrayList<Item> inventory; 

inventory = new ArrayList<Item>();

/// if this list is modified than this.inventory is not changed
public List<Item> getInventory() {
    return new Arraylist<Item>(inventory);
}

/// this code modifies inventory
public void setInventory(ArrayList<Item> inventory){
    this.inventory = inventory;
}

score 0 · Answer 3 · answered Jan 19 '18 at 17:04

When you return direct array list, callers of that list may add/remove elements of it. And you have no control on this list once they get reference of it.

If you return a copy of it, you have your original without any modifications and caller doesn't have any control on it.

score 0 · Answer 4 · answered Jan 19 '18 at 17:07

inventory is defined as a private variable but if you return reference to it external methods, it can be changed by removing or adding new elements.

Also, items are exposed as well, those can also be edited if possible by accessing the ArrayList reference and then iterating over it.

How does returning an arraylist break encapsulation?

4 Answers4