dbluesk
dbluesk

Reputation: 265

Check in constructor vs. set function

this is a very simple question. Suppose I have a class

class A
{
public:
  A(int);
  void setA(int);
private:
  int a;
};

with implementation

A::A(int a_) : a(a_) { }
void A::setA(int a_) { a = a_; }

Let's say I want to avoid having a = 0 but I don't want to throw an exception, I just want to keep a valid object with a = 1 instead in that case (it doesn't matter why right now).

Then I can add an if statement in the constructor and in setA to check whether the parameter is 0, and set it to 1 in that case. Conceptually, this tells me that in that case I should change the constructor, and instead of initializing a there, I should call setA in the constructor. In this way, the code for the check is only written once (keep in mind that this a simple situation but the validation could be more complicated in other situations).

Now, that approach involves having an extra function call in the constructor. Is it more efficient to write the validation code twice? What about if setA were only to be used sporadically?

Upvotes: 1

Views: 141

Answers (2)

Jonathan Mee
Jonathan Mee

Reputation: 38919

I agree with your fervor to prevent code duplication. Just be careful of going overboard, a 1-liner might be overboard.

If you're going to do the check though you should use an implementation file function or a private, static method to do this:

int preventZero(const int a_) { return a_ == 0 ? 1 : a_; }

Then you can use it in your implementation as follows:

A::A(int a_) : a(preventZero(a_)) { }
void A::setA(int a_) { a = preventZero(a_); }

Upvotes: 1

dmsovetov
dmsovetov

Reputation: 299

It's definitely better to call setter with validation in constructor, because code duplication is evil. You can also make it inlined to make sure you are not wasting CPU cycles, but this is a premature optimization, I think.

Upvotes: 1

Related Questions