Software Engineering
java design-patterns api-design code-smell constructors
Updated Wed, 27 Jul 2022 13:00:27 GMT

Should constructors ever be used only for side-effects?

Summary: Why is it wrong to design a constructor only for its side effects, and then to use the constructor without ever assigning its return value to a variable?

I'm working on a project that involves modeling card games. The public API for the game uses an odd pattern for instantiating Rank and Suit objects. Both use a static list to track instances. In a standard deck, there will only ever be four Suit and thirteen Rank objects. When constructing a card, Ranks and Suits are obtained with a static getRank() and getSuit(), which returns the identified item from the static list. Rank and Suit have other static methods such as removeRank() and clear().

What is odd is that there is no static add() method to add a new Rank or Suit to the static list. Instead, this behavior occurs only when a Rank or Suit constructor is invoked via new. Since instances of Rank and Suit are obtained with the static getRank(), the return value from the new constructors is completely irrelevant.

Some sample code might look like this

//set up the standard suits
Suit.clear(); //remove all Suit instances from the static list
new Suit("clubs", 0);  //add a new instance, throw away the returned value
new Suit("diamonds", 1);
new Suit("hearts", 2);
new Suit("spades", 3);
//create a card
Deck theDeck = new Deck()
for (int i = 0; i < 4; i++) {
    for (int j = 0; j < 13; j++) {
        Suit theSuit = Suit.getSuit(i);
        Rank theRank = Rank.getRank(j);
        theDeck.add(new Card(theSuit, theRank));

I've never seen code that didn't assign the object being returned by a constructor. I didn't even think this was possible in Java until I tested it. However, when the issue was raised that the static addSuit() method was missing, the response was

"Objects of type Suit and Rank are created via constructors. The intent is to mimic the behavior of the java.lang.Enum class in which objects are created via constructors and the class maintains the collection of such objects. Current behavior is designed as intended."

Using constructors in this manner smells really wrong to me, and I want to reopen the issue, but I can't seem to find much material in support (coding guidelines, anti-patterns, other documentation). Is this in fact a valid coding pattern that I just haven't come across before? If not, what is a good argument I can make for getting the API changed?

Here is an article that talks about what constructors are expected to do:

We can agree that a constructor always needs to initialize new instances, and we might even agree by now that a constructor always needs initialize instances completely.

Edits: Added additional information about designer's response to issue. Added a few articles that talk about what constructors should and should not do.


Sure, technically this can work, but it violates the so-called Principle of least astonishment, because the typical convention for how constructors are used and what they are good for is quite different. Moreover, it violates the SRP, since now the constructors do two things instead of the one they are usually supposed to do - constructing the object and adding it to a static list.

In short, this is an example for fancy code, but not for good code.