Question

I realize I'm going to get flamed for not simply writing a test myself... but I'm curious about people's opinions, not just the functionality, so... here goes...

I have a class that has a private list. I want to add to that private list through the public getMyList() method.

so... will this work?

public class ObA{
 private List<String> foo;
public List<String> getFoo(){return foo;}
}

public class ObB{
   public void dealWithObAFoo(ObA obA){
     obA.getFoo().add("hello");

   }
}
Was it helpful?

Solution

Yes, that will absolutely work - which is usually a bad thing. (This is because you're really returning a reference to the collection object, not a copy of the collection itself.)

Very often you want to provide genuinely read-only access to a collection, which usually means returning a read-only wrapper around the collection. Making the return type a read-only interface implemented by the collection and returning the actual collection reference doesn't provide much protection: the caller can easily cast to the "real" collection type and then add without any problems.

OTHER TIPS

Indeed, not a good idea. Do not publish your mutable members outside, make a copy if you cannot provide a read-only version on the fly...

public class ObA{
  private List<String> foo;
  public List<String> getFoo(){return Collections.unmodifiableList(foo);}
  public void addString(String value) { foo.add(value); }
}

If you want an opinion about doing this, I'd remove the getFoo() call and add an add(String msg) and remove(String msg) methods (or whatever other functionality you want to expose) to ObA

Giving access to collection always seems to be a bad thing in my experience--mostly because they are virtually impossible to control once they get out. I've taken to the habit of NEVER allowing direct access to collections outside the class that contains them.

The main reasoning behind this is that there is almost always some sort of business logic attached to the collection of data--for instance, validation on addition or perhaps some day you'll need to add a second closely-related collection.

If you allow access like you are talking about, it will be very difficult in the future to make a modification like this.

Oh, also, I often find that I eventually have to store a little more data with the object I'm storing--so I create a new object (only known inside the "Container" that houses the collection) and I put the object inside that before putting it in the collection.

If you've kept your collection locked down, this is a trivial refactor. Try to imagine how difficult it would be in some case you've worked on where you didn't keep the collection locked down...

If you wanted to support add and remove functions to Foo, I would suggest the methods addFoo() and removeFoo(). I ideally you could eliminate the getFoo at together by creating a method for each piece of functionality you need. This make it clear as to the functions a caller will preform on the list.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow