'Collections.unmodifiableCollection' в конструкторе

Question

Время от времени во время проверки кода я вижу конструкторы, подобные этому:

Foo(Collection<String> words) {
    this.words = Collections.unmodifiableCollection(words);
}

Это правильный способ защиты внутреннего состояния класса? Если нет, каков идиоматический подход к созданию правильной защитной копии в конструкторах?

Answer 1

Должно быть, но это не правильно, потому что вызывающая сторона может все еще изменить базовый список.

Вместо того, чтобы переносить список, вы должны сделать защитную копию, например, используя Guava's ImmutableList вместо.

Foo(Collection<String> words) {
    if (words == null) {
       throw new NullPointerException( "words cannot be null" );
    }
    this.words = ImmutableList.copyOf(words);
    if (this.words.isEmpty()) { //example extra precondition
       throw new IllegalArgumentException( "words can't be empty" );
    }
}

Таким образом, правильный способ установить начальное состояние для класса:

1. Проверьте входной параметр для null,
2. Если тип ввода не гарантированно является неизменным (как в случае с Collection), сделайте защитную копию. В этом случае, потому что тип элемента является неизменным (String), подойдет мелкая копия, но если это не так, вам придется сделать более глубокую копию.
3. Выполните любую дополнительную проверку предварительных условий на копии. (Выполнение этого на оригинале может оставить вас открытым для атак TOCTTOU.)

Answer 2

Collections.unmodifiableCollection(words);

создает только оболочку, которую вы не можете изменить words, но это не значит, что words не может быть изменено в другом месте. Например:

List<String> words = new ArrayList<>();
words.add("foo");
Collection<String> fixed = Collections.unmodifiableCollection(words);

System.out.println(fixed);
words.add("bar");
System.out.println(fixed);

результат:

[foo]
[foo, bar]

Если вы хотите сохранить текущее состояние words в неизменяемой коллекции вам нужно будет создать собственную копию элементов из переданной коллекции, а затем обернуть ее Collections.unmodifiableCollection(wordsCopy);

например, если вы хотите сохранить только порядок слов:

this.words = Collections.unmodifiableCollection(new ArrayList<>(words));
// separate list holding current words ---------^^^^^^^^^^^^^^^^^^^^^^

Answer 3

Нет, это не защищает это полностью.

Идиома, которую я люблю использовать, чтобы убедиться, что содержимое является неизменным:

public Breaker(Collection<String> words) {
    this.words = Collections.unmodifiableCollection(
        Arrays.asList(
            words.toArray(
                new String[words.size()]
            )
        )
    );
}

Недостатком здесь является то, что если вы передадите HashSet или TreeSet, он потеряет скорость поиска. Вы можете сделать что-то иное, чем преобразование его в список фиксированного размера, если вы заботитесь о характеристиках хэша или дерева.