Skip to main content
The 2024 Developer Survey results are live! See the results
Tweeted twitter.com/StackSoftEng/status/855407205029482496
added 710 characters in body
Source Link
Brian H.
  • 179
  • 1
  • 5

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() method, 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() method, 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 it would be better to implement this behavior by way of athe static addSuit() method was missing, the response was "Issue Closed. Working as intended".

"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.

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() method, 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() method, 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 it would be better to implement this behavior by way of a static addSuit() method, the response was "Issue Closed. Working 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?

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.

edited body
Source Link
Brian H.
  • 179
  • 1
  • 5

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() method, which returns the identified item from the static list. Rank and Suit have other static methods such as removeRank() and 'clear()`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() method, 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 it would be better to implement this behavior by way of a static addSuit() method, the response was "Issue Closed. Working 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?

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() method, 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() method, 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 it would be better to implement this behavior by way of a static addSuit() method, the response was "Issue Closed. Working 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?

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() method, 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() method, 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 it would be better to implement this behavior by way of a static addSuit() method, the response was "Issue Closed. Working 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?

Source Link
Brian H.
  • 179
  • 1
  • 5

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() method, 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() method, 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 it would be better to implement this behavior by way of a static addSuit() method, the response was "Issue Closed. Working 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?