views:

127

answers:

5

Consider this code snippet:

class MyClass{
    private List myList;
    //...
    public List getList(){
        return myList;
    }
}

As Java passes object references by value, my understanding is that any object calling getList() will obtain a reference to myList, allowing it to modify myList despite it being private. Is that correct?

And, if it is correct, should I be using

return new LinkedList(myList);

to create a copy and pass back a reference to the copy, rather than the original, in order to prevent unauthorised access to the list referenced bymyList?

+7  A: 

I do that. Better yet, sometimes I return an unmodifiable copy using the Collections API.

If you don't, your reference is not private. Anyone that has a reference can alter your private state. Same holds true for any mutable reference (e.g., Date).

duffymo
If you supply an unmodifiable view of the original collection, be aware that any changes in the original list will be reflected in the view. Life would be so much simpler with immutable collections...
Tom Hawtin - tackline
+2  A: 

It depends on what you want.

Do you want to expose the list and make it so people can edit it?

Or do you want to let people look at it, but not modify it?

There is no right or wrong way in this case. It just depends on your design needs.

jjnguy
So, if your design dictates an attribute has private visibility, you should return a copy of the object rather than a simple reference to ensure that it remains encapsulated
chrisbunney
@chris If you design requires that people should not be able to modify your list, then pass a copy. Otherwise pass the original reference.
jjnguy
Yes, sorry, it was a rhetorical comment, I understood your answer ;)
chrisbunney
+1  A: 

There can be some cases when one would want to return the "raw" list to the caller. But in general, i think that it is a bad practice as it breaks the encapsulation and therefore is against OO. If you must return the "raw" list and not a copy then it should be explicitly clear to the users of MyClass.

A: 

Yes, and it has a name.. "Defensive copy". Copying at the receiving end is also recommended. As Tom has noted, behavior of the program is much easier to predict if the collection is immutable. So unless you have a very good reason, you should use an immutable collection.

When Google Guava becomes part of the Java standard library (I totally think it should), this would probably become the preferred idiom:

return ImmutableList.copyOf(someList);

and

void (List someList){
    someList = ImmutableList.copyOf(someList);

This has an added bonus of performance, because the copyOf() method checks whether the collection is already an instance of immutable collection (instanceof ImmutableList) and if so, skips the copying.

Enno Shioji
A: 

I think that the pattern of making fields private and providing accessors is simply meant for data encapsulation. If you want something to be truly private, don't give it accessor methods! You can then write other methods that return immutable versions of your private data or copies thereof.

Jesse