Setters in Constructor – Why Setters are Not Commonly Used in Constructors

access-modifiersconstructorsdesign-patterns

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?

Best Answer

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.

Related Topic