CODEWITHSUNDEEP
javacollections
Eric Rosenberg
Eric Rosenberg

Reputation: 1533

What's the best practice for returning an internal collection in Java?

I'm curious what is considered a better practice when returning a collection of objects from a mutable class:

public class Someclass {

  public List<String> strings;  

  public add(String in){ strings.add(in); }
  public remove(String in) { strings.remove(in); }   

  //THIS
  public List<String> getStrings(){
    return Collections.unmodifiableList(strings);
  }

  //OR THIS
  public List<String> getStrings(){
    return new ArrayList(strings);
  }
}

I always assumed wrapping the internal collection in an Unmodifiable was the best approach since I didn't see a reason to incur the overhead of creating a copy. However, it occurred to me that any changes made to the list internally would be exposed to the client which strikes me as bad.

Upvotes: 12

Views: 4876

Answers (4)

Stephen C
Stephen C

Reputation: 718678

I don't think there is a simple "best practice" answer to this. It really depends on how much of the machine's resources you are willing to spend on preventing violation of the classes data abstraction boundaries. And it depends on what risks you are actually trying to mitigate; e.g. is it simple (non-concurrent) bugs, concurrency bugs, or information leaks.

The options are:

  • Do nothing; i.e. return the collection as-is.
  • Return the collection wrapped in an immutable wrapper class.
  • Return a shallow copy of the collection.
  • Return a deep copy of the collection.

In addition to the direct costs of copying, other performance-related factors are memory usage and garbage generation and the impact on concurrency. For instance, if multiple threads are updating the collection and/or "getting" it, then making a copy of the collection typically involves locking it ... which could potentially make the operation a concurrency bottleneck.

In summary, you need to balance the cost / performance implications against the potential or actual risks and costs of not taking the precautions.

Upvotes: 9

卢声远 Shengyuan Lu
卢声远 Shengyuan Lu

Reputation: 32004

'new ArrayList(strings)' is safer than 'Collections.unmodifiableList(strings)'.

Because Collections.unmodifiableList just forwards the source list.

Sample:

List<String> list1 = someclass.getStrings(); //Use Collections.unmodifiableList(strings) implementation
List<String> list2 = someclass.getStrings(); //Use new ArrayList(strings) implementation

someclass.add("foo");

Now, you'll see list1 is added! list2 not.

Upvotes: 0

Basanth Roy
Basanth Roy

Reputation: 6490

If you are concerned about exposing changes; and your object implements Cloneable, then you can return a Cloned deep copy of your object list back to the invoker.

Upvotes: 0

V a a m Y o b
V a a m Y o b

Reputation: 492

I like to do the latter, but using the .clone() method.

Upvotes: 2

Related Questions