0

在代码审查期间，我不时看到这样的构造函数：

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

这是保护班级内部状态的正确方法吗？如果不是，在构造函数中创建适当的防御性副本的惯用方法是什么？

4

3 回答 3

2

应该是，但它不正确，因为调用者仍然可以修改基础列表。

您应该制作一个防御性副本，而不是包装列表，例如使用 GuavaImmutableList代替。

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攻击。）
于 2016-01-28T20:56:43.720 回答
1 
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 ---------^^^^^^^^^^^^^^^^^^^^^^
于 2016-01-28T21:01:08.827 回答
0

不，这并不能完全保护它。

我喜欢用来确保内容不可变的成语：

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

但是，这里的缺点是，如果您传入 HashSet 或 TreeSet，它将失去速度查找。如果您关心 Hash 或 Tree 特性，您可以做一些事情，而不是将其转换为固定大小的 List。

于 2016-01-28T21:13:59.710 回答