Can and should constructors do more work than merely assigning values to fields? [duplicate]

Question

A typical introductory example to OOP, classes, and constructors is object Car, with properties such as float fuel, bool is_engine_running, etc etc, and a class and constructor definition might be as follows (I'm using a Java-esque syntax here, but it works in any OOP language):

class Car {
    float fuel_remaining;
    bool is_engine_running;
    // ... other fields

    Car(float fuel, bool is_engine_running, ...) {
        this.fuel_remaining = fuel;
        this.is_engine_running = is_engine_running;
        // Other assignments
    }
}

Most examples of constructors I've seen (admittedly not many, as I haven't dug through the sources for large Java/C++ projects yet) tend to do simple this.x = x assignments. I would like to implement a class representing UUIDs (I know there already exists various built-in/third-party libraries dealing with this; this is a custom namespace-based UUID for a client with specific requirements) with the UUID itself internally represented as a 128-bit (or 32-byte) array. I am considering C++'s std::bitset<128> for this, or any equivalent in other languages.

When I initialise the UUID object, I would think that valid constructors would accept, amongst others, the current time, the variables (typically strings) from the namespace, some hardware address (also possibly a string), or even a UUID string itself; for instance, "9fbaea6e-a929-4833-a802-9d64ac432126". Otherwise, I would have to directly provide a bit/byte array for the constructor to do something like this.uuid_bits = uuid_bits, which I feel is rather pointless, because it removes any abstraction whatsoever and leaks the internal bit-array representation to the caller.

Therefore, my question is this: The above UUID class will require some string-parsing, bit-shifting, etc to properly initialise any object while still abstracting away the internal representation. Is it hence an anti-pattern to do more work than merely trivial assignment in the constructor? If not, how might I separate out this 'extra work'? Would I perhaps write and use private static functions/methods to further process the arguments to the constructor, and return the appropriate internal representation, so that the constructor itself only does an assignment?

Answer 1

It very much depends on your class, what it does, how you expect to use it, and what kind of performance optimization you're looking for.

If you're looking for the constructor to trigger some external work, e.g. a cleanup of the file server, that is not appropriate. But I suspect you're thinking more along the lines of calculations of values that are relevant to the currently constructed class' lifetime.

The first approach is to calculate your values once, in the beginning. This means that you don't have to repeat the same calculation whenever the data is requested.

For the examples, I'm going to use taking the first letter of a string as "the calculation". It's of course a trivial amount of effort, but it's merely intended as a simple example of a value calculation of any level of complexity.

public class Person
{
    public string Name { get; private set; }

    public char FirstLetterOfName { get; private set; }

    public Person(string name)
    {
        this.Name = name;
        this.FirstLetterOfName = name[0];
    }
}

This is straightforward, and leads to increased performance on repeated usage, at the cost of initial performance when initializing the object.

The second approach is to defer the execution of the value until it is asked. This means you don't waste time during initialization, especially if you never end up fetching this specific value. But if you fetch the specific value multiple times, you'll lose performance by redoing the same calculation.

public class Person
{
    public string Name { get; private set; }

    public char FirstLetterOfName
    { 
        get { return this.Name[0]; }
    }

    public Person(string name)
    {
        this.Name = name;
    }
}

The third approach tries to have it both ways: deferred execution, and caching the result so the execution is not repeated.

public class Person
{
    public string Name { get; private set; }

    private char? _firstLetterOfName;
    public char FirstLetterOfName
    {
        get
        {
            if(_firstLetterOfName == null)
                _firstLetterOfName = this.Name[0];

            return _firstLetterOfName;
        }
    }

    public Person(string name)
    {
        this.Name = name;
    }
}

This approach maximizes performance both during initialization and during repeated usage, but it comes at the cost of a more complex implementation, which affects readability and maintainability, in a "straw that broke the camel's back" sense.

---

So, depending on which approach you take, you can enhance:

1. Performance during initialization
2. Performance during repeated usage
3. Code readability and maintainability

You can't have it all, but you can have two out of three. Take your pick.

• 1 + 2 => third approach
• 1 + 3 => second approach
• 2 + 3 => first approach

Answer 2

Problems with logic in constructors:

1. It's considered bad practice for constructors to throw exceptions
2. Constructors are not inherited, so you cant mock or dependency inject them
3. Calling code often doesn't control when they are called eg deserialisation or binding
4. They are not optional, so lists of objects or copies etc are problematic

However, in your case you have a reasonably good argument for a constructor. The logic will be a simple function of the input parameters, unlikely to throw exceptions, unlikely to require inheritance or injection etc.

Although, if we check what microsoft does, they go with the static method approach

Guid.NewGuid()

This suffers from lots of the same issues, but again there is a clear spec for Guids that's unlikely to change in a breaking way and there is precedence for such things in Math.Whatever() functions.

Probably the best way to go is to have a builder object

var b = new GuidV3Builder();
var guid = b.Generate(params);

Now you can separate the various versions, have a testable interface, obey SOLID and also have a clear way to implement things like sequential GUIDS