CODEWITHSUNDEEP
javajava-6
James Michael Hare
James Michael Hare

Reputation: 38397

Best way to construct a read-only, empty List?

I'm creating an immutable representation of an "event" in my system, and thus for lists of owners passed in the constructor, I'd like to take a read only view of them. Further, if they pass in null for the list, I'd like to make a read-only empty list in that case.

Now, since Collections.unmodifiableList balks at null, I currently have this:

userOwners_ = Collections.unmodifiableList(userOwners != null 
                                           ? userOwners 
                                           : new ArrayList<String>(0));

But that seems a bit ugly and inefficient. Is there a more elegant way to do this in Java?

Upvotes: 2

Views: 1545

Answers (4)

Stephen C
Stephen C

Reputation: 718916

An equally ugly, but marginally more efficient answer would be

userOwners_ = userOwners != null ? 
                  Collections.unmodifiableList(userOwners) :
                  Collections.emptyList();

However there are a couple of other things to observe.

  1. It appears that at some point, someone has decided to use null to represent an empty list. That is poor design ... and results in the need for special handling. Better to set it to either a new list, or emptyList() if you know the list is always empty.

  2. If you haven't consciously decided that null is the way to represent an empty list, then that null is "unexpected" and you should juts let it throw an NPE so you can track down and fix the cause. (It could be a variable that you have assumed is initialized elsewhere ... but isn't. That's a bug.)

  3. There is some confusion about whether you want a "read-only" list or an "immutable" list:

    • The unmodifiableList() method gives you a list that you cannot modify; i.e. it is "read only". But the original list can still be modified, and those changes will be visible via the "read only" wrapper.
    • If you want an "immutable" list (i.e. one that cannot be changed at all), you need to clone() the original list, and then wrap the clone using unmodifiableList().
    • Neither of these will make the elements of the list (the "owner" objects) immutable (if they are not already immutable).

  4. The identifier userOwners_ is a code style violation in the most widely accepted / used Java style guide.

Upvotes: 3

Daniel Pryden
Daniel Pryden

Reputation: 60957

My preferred way would be using Guava:

this.userOwners = ImmutableList.copyOf(Preconditions.checkNotNull(userOwners));

Like tackline's answer, this also throws an exception rather than silently translating null into the empty list.

Unlike the other answers here, using ImmutableList.copyOf() ensures that the caller can't pass you a list that they can later mutate.

Upvotes: 1

Kevin Day
Kevin Day

Reputation: 16383

The resultant userOwners_ will still be mutable - any changes to userOwners will be part of userOwners_.

The right way to do this if you really want that member variable to be immutable:

private final List<String> userOwners;

public MyObject(List<String> userOwners){
  this.userOwners = userOwners != null ? Collections.unmodifiableList(new ArrayList<String>(userOwners)) : Collections.emptyList();
}

As a minor point, your member variable naming isn't following Java style guidelines (userOwners_ is strange to those of us who read Java code on a regular basis)

To expand on what another poster wrote: Think really, really hard before you accept a null input to a public method (without throwing NPE). This sort of behavior can hide bugs - much better to fail fast and force the caller to think about what they are doing.

Upvotes: 1

Tom Hawtin - tackline
Tom Hawtin - tackline

Reputation: 147164

Collections.emptyList(). But seriously, null should NPE.

Upvotes: 6

Related Questions