I'm writing a physics simulation (Ising model) in C++ that operates on square lattices. The heart of my program is my Ising class with a constructor that calls for the row and column dimensions of the lattice. I have two other methods to set other parameters of the system (temperature & initial state) that must get called before evolving the system! So, for instance, a sample program might look like this

If the system.set_*() methods aren't called prior to system.step(), system.step() throws an exception alerting the user to the problem. I implemented it this way to simplify my constructor; is this bad practice?

No, it's not poor form, any more than absolute beginner's code is poor form. It's like that absolute beginner's code. With some more experience you'll learn about class invariants and RAII (both of which requires single construction step), and you'll learn about techniques to support simple constructor usage, such as NPI.
–
Cheers and hth. - AlfJan 11 '11 at 14:15

7 Answers
7

It is recommended to put all mandatory parameters in the constructor whenever possible (there are exceptions of course, but these should be rare - I have seen one real-world example so far). This way you make your class both easier and safer to use.

Note also that by simplifying your constructor you make the client code more complicated instead, which IMO is a bad tradeoff. The constructor is written only once, but caller code may potentially need to be written many times more (increasing both the sheer amount of code to be written and the chance of errors).

You may also consider a IsingInitialState class that your user must fill up and pass as an argument to your Ising class.
–
Alexandre C.Jan 11 '11 at 12:41

@Alexandre C., indeed, if there are too many parameters, it is worth introducing a Parameter Object. But IMHO 4 parameters is not that big a problem yet.
–
Péter TörökJan 11 '11 at 12:43

1

+1. Note that the constructor is implemented only once, but may be called from many different places, thus incrementing the probability of error.
–
David Rodríguez - dribeasJan 11 '11 at 12:52

Thanks! This is almost exactly why I thought it might be a bad idea (with much improved wording). Though I never though of adding a separate class to initialize...I'll have to try that!
–
Connor GlosserJan 11 '11 at 13:04

Not at all, IMO. I face the same thing when loading data from external files. When the objects are created (ie, their respective ctors are called), the data is still unavailable and can only be retrieved at a later stage. So I split the initialisation in different stages:

constructor

initialisation (called by the framework engine when an object is activated for the first time)

activation (called each time an object is activated).

This is very specific to the framework I'm developing, but there is no way to deal with everything using just the constructor.

However, if you know the variables at the moment the ctor is called, it's better not to complicate the code. It's a possible source of headaches for anyone using your code.

IMO this is poor form if all of these initialization steps must be invoked every time. One of the goals of well-designed software is to minimize the opportunities to screw up, and having multiple methods which must be invoked before an object is "usable" simply makes it harder to get right. If these calls were optional then having them as separate methods would be fine.

The entire point in a class is to present some kind of abstraction. As a user of a class, I should be able to assume that it behaves like the abstraction it models.

And part of that is that the class must always be valid. Once the object has been created (by calling the constructor), the class must be in a meaningful, valid state. It should be ready to use. If it isn't, then it is no longer a good abstraction.

If the initialization methods must be called in a specific order then I would wrap the call to them in their own method as this indicates that the methods are not atomic on their own so the 'knowledge' of how they should be called should be held in one place.

I'd say that setting the initial conditions should be separate from the constructor if you plan to initialize and run more than one transient on the same lattice.

If you run a transient and stop, then it's possible to move setting the initial conditions inside the constructor, but it means that you have to pass in the parameter values in order to do this.

I fully agree with the idea that an object should be 100% ready to be used after its constructor is called, but I think that's separate from the physics of setting the initial temperature field. The object could be fully usable, yet have every node in the problem at the same temperature of absolute zero. A uniform temperature field in an insulated body isn't of much interest from a heat transfer point of view.