3

Some what of a followup to this question.

Suppose I have a Player class with a dictionary call attributes that holds the Health and Mana of the player. In this example, I'll use Python, but my question is language independent as you'll see when you keep reading.

In the Player constructor, is it okay to add elements to the dictionary?

Example:

class Player:

    def __init__(self, name, health, mana):
        self._name = name
        # Using type hints to indicate the types the dictionary contains
        self._attributes: Dict[str,int] = {}
        self._attributes["Health"] = health
        self._attributesplayer_details["Mana"] = mana

The reason is, it will take a lot of validation to ensure that a correct dictionary is passed to the constructor.

Consider I have to check for the following:

  • The dictionary contains the correct number of elements, in this case 2
  • Both Health and Mana are in the dictionary
  • The values are set correctly, in this case both are set to 100 at the beginning of the game.

In the above example, I simply have to check that Health and Mana don't exceed 100. How it's being stored is of no concern to any outside code.

Does the example violate the guideline that a constructor shouldn't do work? If so, how should I correct it?

Improve this question
2
  • 3
    This is just initialization, so it's okay. It would not be okay if the constructor added an item to a dictionary somewhere else (not part of the object being constructed).
    – user253751
    Jun 21, 2018 at 0:19
  • If you don't initialize where you initialize when will you initialize ? That's why constructors exist. The boundaries of the initialization is what you should be concerned about, along with how long will it take and if it can throw an exception or not. That being said, @immibis answer is perfect.
    – Machado
    Jun 21, 2018 at 13:54

1 Answer 1

Reset to default
3

This is just more initialization, so it's okay. The point of a constructor to set up the object. This is setting up the object, so it's fine.

When they say "a constructor shouldn't do work" they mean the constructor shouldn't do anything other than setting up the object. For example, the following things are not appropriate for constructors and will eventually cause you headaches down the line:

  • Adding an item to a dictionary somewhere else (not part of the object being constructed).
  • Creating a file
  • Initializing some hardware
  • Communicating with a remote system (this may be okay if the remote system's state is not affected)
  • Displaying a message on the screen
  • Waiting for the user to click a button
Improve this answer
6
  • Could you elaborate on how Creating a file, Initializing some hardware, or Communicating with a remote system will cause problems?
    – user949300
    Jun 21, 2018 at 5:34
  • 5
    In the right context, creating a file or initializing hardware, seem perfectly reasonable things to me. For example, if a class is essentially a wrapper for some data held on disk, then it would make sense that creating an object would create the file if it doesn't already exist. Otherwise, the object is initialised into some invalid state, which would then require two-stage initialisation (using a separate member function to initialize the file), which is rather nasty.
    – Simon B
    Jun 21, 2018 at 8:45
  • 3
    @user949300 - To me anyways, it seems that if your constructor is performing these actions, it probably doing too much and violating SRP. For example, why can't you create the file and pass it to the constructor? Communicating with a remote sever is the same, make your connection with the proper settings, and pass that to the class that will use it. I'm in no way an expert, and what I'm saying could be incorrect, but that's just the way it looks to me, the constructor has too many responsibilities.
    – user306112
    Jun 21, 2018 at 14:37
  • 1
    @Sveta agree on SRP concerns. But, at some point something with that "responsibility" has to create the File or connect to the remote server. And that can perhaps be done in a constructor, as in SimonB's comment. For that reason I'm reluctantly downvoting this answer.
    – user949300
    Jun 21, 2018 at 19:54
  • @user949300 I think I understand where you are coming from, but to me the answer doesn't say you can't, just that doing those things are probably inappropriate for a constructor and doing those things in the constructor has a higher chance of being problematic. To me, the context of the answer makes it clear it's not talking about creating a file in the context of a file wrapper class as SimonB's comment suggests (which may fall into that 'appropriate' case for a constructor to do that, again as SimonB is suggesting). My point: the answer isn't suggesting all the examples are forbidden.
    – Cole Ole
    Jun 21, 2018 at 21:44

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.