Best practice concerning private arraylist and other such constructions

Question

Hello all,

I have the following class which I'm fighting over how to implement. The question is whether or not it makes sense to have a private collection item, in this case an arraylist, a a member. I am well aware it is considered best practice to have getters and setters for any class members, but in this case having a getter and setter would require re-implementing (or rather duplicating) large amounts of the ArrayList functionality.

Example class:

public class Email()
{
    private ArrayList<String> To;
    private ArrayList<String> Cc;
    private ArrayList<String> Bcc;

    ...
}

I assume the answer is I should indeed implement getters and setters for these arrays? Is there some approach I haven't thought of regarding handling this type of situation? The easy solution to managing these lists is to set the private modifiers to public and check the array data is valid on calling methods, but is this right? Can anyone point me in the direction of other SO questions, design patterns etc I should consider?

Answer 1

Actually in that case I think it would be better to not provide a direct getter/setter combination.

I would rather approach it the following way:

1. Change the type to List<String> (best practice to use Interfaces on fields.)
2. Initialize an empty ArrayLists in the constructor.
3. Provide delegates for add, remove, hasTo/Cc/Bcc.

In that way you could decorate each collection with additional functionality, e.g. making sure the String contains a valid recipient information.

public void addTo(String recipient){
  // validate recipient
  this.to.add(recipient);
}

public String removeTo(String recipient){
  return this.to.remove(recipient);
}

public boolean hasTo(){
  return this.to.size() > 0;
}

EDIT

I forgot to mention that a getter makes sense since you need that when processing the Email. But look at the other answer on considering to returning an unmodifiable List.

Answer 2

I am well aware it is considered best practice to have getters and setters for any class members.

I disagree. Only expose what you need to! Try to think in terms of ADTs (abstract Data Types). In other words, create a layer of abstraction from the internal ArrayLists. Anyone using your class should not know you are internally using ArrayLists. Unless absolutely necessary, you should not provide a getter for your ArrayLists. If you feel like you need these, think about returning a List and using Collections.unmodifiableList() to prevent modification through a getter.

Answer 3

I am well aware it is considered best practice to have getters and setters for any class members.

That is incorrect.

The relevant "best practice" advice is NOT to expose a classes state as fields.

If you do need to make the state visible, getters and setters are one option, but not necessarily the most appropriate option. In deciding which of the various options is best, you need to consider questions like:

• Is the state that I'm exposing an internal implementation detail of the ADT, or a reference to a separate data entity?

• If I expose an internal implementation detail, how do I prevent something depending on it, or (worse still) changing it in potentially disruptive ways?

Well implemented getters and setters can address these concerns; e.g. by returning copies of collections, or unmodifiable wrappers, and/or by copying collection arguments. But wrapper methods can do the same, often with less runtime overheads.

The other thing to consider is whether there are performance imperatives that make it necessary to eschew "best practice" design principles and deliberately make the ADT "leaky".