2

I was reading this page about using the new keyword in the constructor, and I was wondering if it applies to copy constructors for collections.

Suppose I have a Book class and a collection to store a set of authors.

class Book {

    private String title;
    private Set<Author> authorsSet;

    Book (title, authors){
        this.title = title
        this.authorsSet = Collections.unmodifiableSet(new HashSet<Author>(authors)) 
    }
}

It's a good idea to make a defensive copy so that client code cannot change the internals of authorsSet.

Is calling this.authorsSet = Collections.unmodifiableSet(new HashSet<Author>(authors)) a violation of the guideline that a constructor shouldn't call the new keyword? If so, how do we make a defensive copy without the new keyword?

Improve this question
5
  • 2
    Specifically the highest voted answer directly addresses your question. Commented Aug 31, 2018 at 22:20
  • 1
    "Doing something such as initializing an empty private collection in your constructor is fine, and injecting it would be absurd" Commented Aug 31, 2018 at 22:23
  • @Ewan - It's not an empty private collection, it's actually copying items from one set to another. The constructor is actually doing work with the set. Commented Aug 31, 2018 at 22:59
  • It's not doing anything except getting your object into a valid initial state - it's not work. Commented Sep 1, 2018 at 5:34
  • 2
    Sveta, no offense, but you are often asking question of the form "here is a braindead cargo cult rule, should I follow it or should I start thinking by myself", and the answer you got in the past from different people was always "start thinking by yourself". Robert Harvey describes this as "there is no best practice (=braindead rule), only solutions to requirements". So don't expect anything different this time. Commented Sep 1, 2018 at 6:23

1 Answer 1

Reset to default
8

If in a constructor of class A another object of class B is constructed by new, then A has a hard dependency on B, which makes it impossible to test A isolated from B, which can be resolved by injecting a B instance into A through a constructor parameter of some interface type which B implements. But that is only a necessary

  • if you actually want or need to unit test A isolated from B.

In the shown case, that would mean to unit test Book isolated from the class HashSet - which is probably not what you want or need. Collection classes like HashSet can be treated as elements of your framework or programming languages (actually, in some languages like Perl or Python they are elements of the language). They are lightweight, fast, can be assumed to be bugfree for all practical purposes, and often most your code depends on them anyway. So it typically makes no sense to break a dependency on them.

Improve this answer