Why did it not become a common pattern to use setters in the constructor?

softwareengineering.stackexchange https://softwareengineering.stackexchange.com/questions/329806

Question

Accessors and modifiers (aka setters and getters) are useful for three main reasons:

  1. They restrict access to the variables.
    • For example, a variable could be accessed, but not modified.
  2. They validate the parameters.
  3. They may cause some side effects.

Universities, online courses, tutorials, blog articles, and code examples on the web are all stressing about the importance of the accessors and modifiers, they almost feel like a "must have" for the code nowadays. So one can find them even when they don't provide any additional value, like the code below.

public class Cat {
    private int age;

    public int getAge() {
        return this.age;
    }

    public void setAge(int age) {
        this.age = age;
    }
}

That been said, it is very common to find more useful modifiers, those which actually validate the parameters and throw an exception or return a boolean if invalid input has been supplied, something like this:

/**
 * Sets the age for the current cat
 * @param age an integer with the valid values between 0 and 25
 * @return true if value has been assigned and false if the parameter is invalid
 */
public boolean setAge(int age) {
    //Validate your parameters, valid age for a cat is between 0 and 25 years
    if(age > 0 && age < 25) {
        this.age = age;
        return true;
    }
    return false;
}

But even then, I almost never see the modifiers been called from a constructor, so the most common example of a simple class I face with is this:

public class Cat {
    private int age;

    public Cat(int age) {
        this.age = age;
    }

    public int getAge() {
        return this.age;
    }

    /**
     * Sets the age for the current cat
     * @param age an integer with the valid values between 0 and 25
     * @return true if value has been assigned and false if the parameter is invalid
     */
    public boolean setAge(int age) {
        //Validate your parameters, valid age for a cat is between 0 and 25 years
        if(age > 0 && age < 25) {
            this.age = age;
            return true;
        }
        return false;
    }

}

But one would think that this second approach is a lot safer:

public class Cat {
    private int age;

    public Cat(int age) {
        //Use the modifier instead of assigning the value directly.
        setAge(age);
    }

    public int getAge() {
        return this.age;
    }

    /**
     * Sets the age for the current cat
     * @param age an integer with the valid values between 0 and 25
     * @return true if value has been assigned and false if the parameter is invalid
     */
    public boolean setAge(int age) {
        //Validate your parameters, valid age for a cat is between 0 and 25 years
        if(age > 0 && age < 25) {
            this.age = age;
            return true;
        }
        return false;
    }

}

Do you see a similar pattern in your experience or is it just me being unlucky? And if you do, then what do you think is causing that? Is there an obvious disadvantage for using modifiers from the constructors or are they just considered to be safer? Is it something else?

Was it helpful?

Solution

Very general philosophical reasoning

Typically, we ask that a constructor provide (as post-conditions) some guarantees about the state of the constructed object.

Typically, we also expect that instance methods can assume (as pre-conditions) that these guarantees already hold when they're called, and they only have to make sure not to break them.

Calling an instance method from inside the constructor means some or all of those guarantees may not yet have been established, which makes it hard to reason about whether the instance method's pre-conditions are satisfied. Even if you get it right, it can be very fragile in the face of, eg. re-ordering instance method calls or other operations.

Languages also vary in how they resolve calls to instance methods which are inherited from base classes/overridden by sub-classes, while the constructor is still running. This adds another layer of complexity.

Specific examples

  1. Your own example of how you think this should look is itself wrong:

    public Cat(int age) {
    //Use the modifier instead of assigning the value directly.
    setAge(age);
}

    this doesn't check the return value from setAge. Apparently calling the setter is no guarantee of correctness after all.

  2. Very easy mistakes like depending on initialization order, such as:

    class Cat {
  private Logger m_log;
  private int m_age;

  public void setAge(int age) {
    // FIXME temporary debug logging
    m_log.write("=== DEBUG: setting age ===");
    m_age = age;
  }

  public Cat(int age, Logger log) {
    setAge(age);
    m_log = log;
  }
};

    where my temporary logging broke everything. Whoops!

There are also languages like C++ where calling a setter from the constructor means a wasted default initialization (which for some member variables at least is worth avoiding)

A simple proposal

It's true that most code isn't written like this, but if you want to keep your constructor clean and predictable, and still reuse your pre- and post-condition logic, the better solution is:

class Cat {
  private int m_age;
  private static void checkAge(int age) {
    if (age > 25) throw CatTooOldError(age);
  }

  public void setAge(int age) {
    checkAge(age);
    m_age = age;
  }

  public Cat(int age) {
    checkAge(age);
    m_age = age;
  }
};

or even better, if possible: encode the constraint in the property type, and have it validate its own value on assignment:

class Cat {
  private Constrained<int, 25> m_age;

  public void setAge(int age) {
    m_age = age;
  }

  public Cat(int age) {
    m_age = age;
  }
};

And finally, for complete completeness, a self-validating wrapper in C++. Note that although it's still doing the tedious validation, because this class does nothing else, it's relatively easy to check

template <typename T, T MAX, T MIN=T{}>
class Constrained {
    T val_;
    static T validate(T v) {
        if (MIN <= v && v <= MAX) return v;
        throw std::runtime_error("oops");
    }
public:
    Constrained() : val_(MIN) {}
    explicit Constrained(T v) : val_(validate(v)) {}

    Constrained& operator= (T v) { val_ = validate(v); return *this; }
    operator T() { return val_; }
};

OK, it isn't really complete, I've left out various copy and move constructors and assignments.

OTHER TIPS

When an object is being constructed, it is by definition not fully formed. Consider how the setter you provided:

public boolean setAge(int age) {
    //Validate your parameters, valid age for a cat is between 0 and 25 years
    if(age > 0 && age < 25) {
        this.age = age;
        return true;
    }
    return false;
}

What if part of the validation in setAge() included a check against the age property to ensure that the object could only increase in age? That condition might look like:

if (age > this.age && age < 25) {
    //...

That seems like a pretty innocent change, since cats can only move through time in one direction. The only problem is that you can't use it to set the initial value of this.age because it assumes that this.age already has a valid value.

Avoiding setters in constructors makes it clear that the only thing the constructor is doing is setting the instance variable and skipping any other behavior that happens in the setter.

Some good answers here already, but noone so far seemed to have noticed you are asking here two things in one, which causes some confusion. IMHO your post is best seen as two separate questions:

  1. if there is a validation of a constraint in a setter, should it not also be in the constructor of the class to make sure it cannot be bypassed?

  2. why not call the setter directly for this purpose from the constructor?

The answer to first questions is clearly "yes". A constructor, when it does not throw an exception, should leave the object in a valid state, and if certain values are forbidden for certain attributes, then it makes absolutely no sense to let the ctor circumvent this.

However, the answer to the second question is typically "no", as long as you do not take measures to avoid overriding the setter in a derived class. Therefore, the better alternative is to implement the constraint validation in a private method which can be reused from the setter as well as from the constructor. I am not going here to repeat @Useless example, which shows exactly this design.

Here's a silly bit of Java code that demonstrates the kinds of issues you can run into with using non-final methods in your constructor:

import java.util.regex.Pattern;

public class Problem
{
  public static final void main(String... args)
  {
    Problem problem = new Problem("John Smith");
    Problem next = new NextProblem("John Smith", " ");

    System.out.println(problem.getName());
    System.out.println(next.getName());
  }

  private String name;

  Problem(String name)
  {
    setName(name);
  }

  void setName(String name)
  {
    this.name = name;
  }

  String getName()
  {
    return name;
  }
}

class NextProblem extends Problem
{
  private String firstName;
  private String lastName;
  private final String delimiter;

  NextProblem(String name, String delimiter)
  {
    super(name);

    this.delimiter = delimiter;
  }

  void setName(String name)
  {
    String[] parts = name.split(Pattern.quote(delimiter));

    this.firstName = parts[0];
    this.lastName = parts[1];
  }

  String getName()
  {
    return firstName + " " + lastName;
  }
}

When I run this, I get a NPE in the NextProblem constructor. This is a trivial example of course but things can get complicated quickly if you have multiple levels of inheritance.

I think the bigger reason this didn't become common is that it makes the code a good bit harder to understand. Personally, I almost never have setter methods and my member variables are almost invariably (pun intended) final. Because of that, the values have to be set in the constructor. If you use immutable objects (and there are many good reasons to do so) the question is moot.

In any event, it's a good goal to reuse the validation or other logic and you can put it into a static method and invoke it from both the constructor and the setter.

It depends!

If you're performing simple validation or issuing naughty side-effects in the setter that need to be hit every time the property is set: use the setter. The alternative is generally to extract the setter logic from the setter and invoke the new method followed by the actual set from both the setter and the constructor -- which is effectively the same as using the setter! (Except now you're maintaining your, admittedly small, two-line setter in two places.)

However, if you need to perform complex operations to arrange the object before a particular setter (or two!) could successfully execute, don't use the setter.

And in either case, have test cases. Regardless of which route you go, if you have unit tests, you'll have a good chance of exposing the pitfalls associated with either option.

I'll also add, if my memory from that far back is even remotely accurate, this is the sort of reasoning I was taught in college too. And, it's not at all out of line with successful projects I've had the privy to work on.

If I had to guess, I missed a "clean code" memo at some point that identified some typical bugs can that arise from using setters in a constructor. But, for my part, I haven't been victim to any such bugs ...

So, I'd argue that this particular decision doesn't need to be sweeping and dogmatic.

Licensed under: CC-BY-SA with attribution
scroll top