Which option is best, when you have an attribute that is a list?

Question

I have a class containing a list, for example:

class User{
   ....
   List<String> cards
   ...
}

What's the best way to provide access to this member?

List<String> getCards()

or

String getCard(int index)

I am tempted by the second option so that I can avoid a null pointer exception like this:

getCards().get(0) //if getCards() returns null

But then I also wonder if this is something that should not be the responsibility of the class.

Answer 1

This answer is written from C# perspective. Please feel free to make edits to replace C# interfaces with Java equivalents, or to mention discrepancies or lack of equivalents between the two languages where appropriate.

---

Evan's answer has value, in terms of keeping things simple (KISS).

---

Before going into Robert Bräutigam's answer, let's look at how this task is traditionally handled by Java practitioners _{(of which I'm not)}:

• A Card object or an AbstractCard interface (instead of using String),
• An UserCardConsumer interface or abstract class (or CardConsumer for short), which defines methods that accept either one card at a time, or entire collection of cards at once. This is meant to be inheritable (be implemented or extended).
• Any business logic that wants to "consume" cards from a User will need to implement an inner class that inherits from UserCardConsumer, which will consume the cards when its method is called.

Given the additional amount of traditional code (of which Java is famous for), Robert Bräutigam reiterates the need to think about the bare necessary needs of the application. If you can encapsulate the entire logic that operates on that Card collection in one class, go for it.

---

Decide what User class needs to be. Typically, more than one needs exist.

• An interface?
• A mutable data transfer object?
• A data model (as in MVC, MVVM etc)
• An immutable data object?
• An actor?
  • An actor (as in actor model) basically handles the entire business logic within itself (not necessary encapsulating the whole application; just its share of responsibilities), typically not having to reveal most of its information to outsiders.

---

If you expose the getCard(int index) method, you might also need to expose an int getCardCount() method, so that the callers can use a for-loop.

---

If you are unsure about exposing a mutable List<String> member (i.e. sharing the reference to the internal member with the caller), for various reasons, here are some options:

• IReadOnlyList<> (immutable; otherwise behaves like a list)
• IList<> (maybe-mutable; otherwise behaves like a list)
  • Why I said "maybe-mutable"? Many immutable list implementations offer an IList interface as well, but their mutators will throw an exception.
  • LSP (Liskov) and ISP (Interface Segregation) would suggest that, if the list is always going to be immutable, exposing IReadOnlyList<> is better than IList<>.
  • However, if the object or interface must allow flexibility in the underlying implementation, IList<> would still be the choice.
• ICollection<> ... This one does exactly what you want:
  • Has a Count property, so that you can do user.Cards.Count
  • Has an indexer, so that you can do user.Cards[2]
  • Enumerable (iterable)
  • Basically, ICollection<> is a better choice over IEnumerable<>.
  • Note that ICollection<> does not have an Add(T) method.
    • This may not be an issue for you, since you can put this method on the User class. This is even more preferable, since this allows User to be notified of changes to the list, so that it can perform additional actions to maintain the object invariants if necessary.

---

Issues with staying consistent and synchronized.

For simple applications this may not be a big issue.

For slightly more involved applications, a mutable data transfer object can return a snapshot (copied) list of items. Modification of the list afterwards would not be synchronized to the copied list that was returned earlier.

---

Preventing null exceptions.

My typical approach is as follows.

• Mark the internal List member readonly. This means the same list will always be used (and reused), and will survive modifications and item removals. This means the reference to this list will always be the same.
• Inside the User constructor, initialize this List member. Combined with the readonly keyword, this ensures the reference to this list will always be same and non-null.
• Expose this member as an IList<>. or IReadOnlyList<>
• Operations, such as destructive and non-destructive ones, are carried out on this List.

Answer 2

It is worth noting, that if you are interested in object-orientation and maintainable designs, you should not expose that list at all.

Of course there are rare exceptions, but in general object-orientation is about hiding the data, and giving the object behavior to work with that data.

Every time you expose data or a pure data structure, you essentially lose control over that data, which is really bad for maintenance. Instead of maintaining a meaningful behavior you now have to maintain an arbitrary (that means technical) data interface.

So "the best" option, if there is such a thing, is to look at what the whole application needs and choose a method from that pool of behavior, instead of looking at it just locally and think about what data you would want to have as a developer.

Answer 3

Gona chime in on this one

class User{
   ....
   public List<String> cards {get;set}
   ...
}

Is the most common solution for a good reason.

If you need to provide access to the list then this is just a struct defined as a class. There's no benefit in hiding the data or the methods on List and it's a lot of extra work to do that.

Now if you can hide the list completely any only expose methods on your object then yeah that's great, you are doing OOP, but most classes are just used as structs.

Another option is to make the class immutable. Java provides the ImmutableList<> to help you