3

My program models a sport tournament which has one or many events or categories. The class Event has members like players or courts as a List, as well as some dictionaries using the class Map.

So far accessing publicly those members can be done through their respective getters, but I realized this is a bad practice because it allows modifications through the collection object retrieved with the getter. And I wouldn't want this.

I would like to limit the set of operations allowed on these members and avoid the situation described above.

One might think just declaring these members as final would do, but it doesn't. These members can still be modified, just instead of freely doing that directly with the native List or Map methods, it's done via my classes' methods.

So let's say I clamp the collections set of operations allowed on these collections and I make wrapping classes for my lists and maps, so instead of having this:

private List<Player> players;
private List<Court> courts;

private Map<Player, List<Timeslot>> playersUnavailable;

I could have something like:

private CustomList<Player> players;
private CustomList<Court> courts;

private CustomMap<Player, List<Timeslot>> playersUnavailable;

The name of the classes would probably need to be different, I just can't think of something better at the moment.

Say we just want reader methods for these classes. So we could have a method to get the size and another to get a particular element of the internal collection:

public class CustomList<T> {
    private final List<T> list;
    public CustomList<T>(List<T> l) { list = l; }
    public int size() { return list.size(); }
    public int get(int i) { return list.get(i); }
}

However, now I can't iterate through items of a collection like I normally would:

for (Player player : players)
    System.out.println(player);

Although I could use get() and size() but it feels a bit rudimentary.

What could I do to make these collections iterable but preserving the immutability rules my design has? (As I said it's not strictly immutability, since you can do it using the pertinent methods)

Overall, should I rethink my design differently? Does what I'm trying to do make sense? Could I achieve it differently, in a better way?

I guess I could say what I am looking for are read-only collections, am I?

dabadaba
  • 2,216
  • 6
  • 25
  • 35

2 Answers2

5

It appears you have two requirements here:

  1. Event exposes collections, but callers cannot modify those collections.

  2. Event can modify its collections, but only through the Event class.

It sounds like you need to use immutable collections, for example, java.util.Collections.unmodifiableList(List list). However, you do not want to store an immutable collection as the state in Event. It could work like this:

public class Event {

  private List<Player> players;

  public List<Player> getPlayers() {
    return Collections.unmodifiableList(players);
  }

  public void addPlayer(Player p) {
    players.add(p);
  }
}

This allows modifying the collections through Event, while still allowing callers to get an immutable reference suitable for iteration. The call to Collections.unmodifiableList(players) wraps the list players and does not copy it.

If this needs to be thread-safe then there are additional concerns to address here, but that was not a requirement. It sounds more like your Event class needs to handle invariants and data integrity between multiple collections that model the same objects. That concern should easily be addressable using this technique.

  • [Writing immutable collections correctly is hard.](http://programmers.stackexchange.com/a/171298/1204) (see the comments below my answer) – Robert Harvey Apr 20 '16 at 00:09
  • I would definitely not provide mutating operations on the wrapper if immutability is desired. But then you essentially have whatever nested class the `Collections` class returns for its `immutableXYZ()` methods anyway. –  Apr 20 '16 at 02:21
  • This leads to the same problem I mentioned about using the `final` modifier. I said **I do want to be able to modify the internal collections**, just **not** with the native collections' methods, but with the pertinent method defined in the class itself. – dabadaba Apr 20 '16 at 07:37
  • 1
    @dabadaba from your question: "I guess I could say what I am looking for are read-only collections, am I?" based on your comment on my answer, I think your question is unclear. Perhaps it would help if you edit your question to resolve this ambiguity? I read the question as you wanted immutable collections. –  Apr 20 '16 at 14:41
  • @dabadaba I think I get it now, please see the edit to my answer. I would like to reiterate that **this is not thread-safe** which is often a reason for desiring immutability. If you just want to ensure data integrity in a single-threaded situation this will work. If it must be thread-safe, that is a whole different issue and will greatly complicate your design. –  Apr 20 '16 at 15:11
  • @Snowman I am not concerned about thread safety. This will be for single-threaded purposes. However, doing it the way you suggested, now I will get a **copy** of the list, right? That's undesirable because I do a lot of comparisons and checks for containment (in the sense that the list contains an object) and now the references would change. If the question is unclear though, I might just create a new question trying to explain myself properly, and adding some more examples. – dabadaba Apr 20 '16 at 17:49
  • 1
    No, `Collections` provides _wrappers_ for collections, those `unmodifiableXYZ()` methods do _not_ copy anything. –  Apr 20 '16 at 19:01
0

You have two choices, other than rolling your own -- slap a facade over a modifiable list (like UnmodifiableList) that doesn't allow modification through that interface, or use one of the available third party libraries that is truly immutable. The problem with the facade approach is that someone (say, a subclass) could modify the underlying list, breaking the contract implied by presenting an UnmodifiableList. An example of the latter is Guava's ImmutableList which is truly immutable, inherently making a defensive copy when initialized from a List.

Hank D
  • 101
  • 1