Should I copy list both in constructor and in getter?

I have a simple immutable class:

public class ColumnsWrapper implements Columns {
    private final List columnNames;

    public ColumnsWrapper(List columnNames) {
        this.columnNames = new ArrayList(columnNames);
    }

    @Override
    public List getNames() {
        return new ArrayList(columnNames);
    }

}

Should I always save a copy of passed list in constructor and return copy of saved list in getter?

This class from library project and can be used in several projects in the future.

The purpose of this class is only to store immutable list of column names.

It depends on what the goal is. Can the list never change after construction? Can it change but only the ColumnWrapper can do so? Or can it change and everyone is allowed to do that?

If the list can not change after construction, consider using an ImmutableList (from google's guava). You should then declare the field columnNames and the return value of getNames() to be of that type. You then either create an immutable list during construction or use that type for the constructor argument as well.

If the list can change after construction (but only ColumnsWrapper can do so), the getter should return an unmodifiableList. Note that this will cause exceptions if the client of your class tries to manipulate the list. You should then also copy during construction (as you currently do).

In any way should you document the behavior with comments on the respective public members (i.e. the constructor and the getter).

Edit

Ok, so the class has to be immutable. As @rolfl explains, it can be subclassed so this is not yet the case. You can either make the class final or make the constructor private and provide a static factory method.

Furthermore you have to make sure, that the list can not be modified. The easiest and most intention revealing way I know of is the ImmutableList I mentioned above. Another solution would look like this:

public class ColumnsWrapper implements Columns {

    private final List columnNamesUnmodifiable;

    public ColumnsWrapper(List columnNames) {
        List columnNamesCopy = new ArrayList<>(columnNames);
        columnNamesUnmodifiable = Collections.unmodifiableList(columnNamesCopy);
    }

    // OPTION A

    @Override
    public List getNamesUnmodifiable() {
        return columnNamesUnmodifiable;
    }

    // OPTION B

    @Override
    public Iterable getNamesUnmodifiable() {
        return columnNamesUnmodifiable;
    }

}

Note that I changed the name to inform callers that they will get an unmodifiable instance.

I also provided another additional option (you have to choose one) with a different return type. If you are sure that callers will only iterate over the returned instance (as is often the case) the iterable will suffice. But it can not be used to add methods and since removal is an optional method (i.e. many iterators support no removal) it better conveys immutability.

I any way the interface documentation should also make that clear.

StackExchange Code Review Q#68196, answer score: 8