Is it better allow a function to throw or throw in the constructor?

Question

I think, it is easier explain using an example. Let's take a class that model the speed of a Formula 1 car, the interface may look something like:

class SpeedF1
{
public:
  explicit SpeedF1(double speed);
  double getSpeed() const;
  void setSpeed(double newSpeed);
  //other stuff as unit 
private:
  double speed_;
};

Now, negative speed are not meaningful in this particular case and neither value greater then 500 km/h. In this case the constructor and the setSpeed function may throw exceptions if the value provide is not within a logical range.

I can introduce an extra layer of abstraction and insert a extra object instead of double. The new object will be a wrapper around the double and it will be construct and never modify. The interface of the class will be changed to:

class ReasonableSpeed
{
public:
  explicit ReasonableSpeed(double speed); //may throw a logic error
  double getSpeed() const;
  //no setter are provide
private:
  double speed_;
};

class SpeedF1
{
public:
  explicit SpeedF1(const ReasonableSpeed& speed);
  ReasonableSpeed getSpeed() const;
  void setSpeed(const ReasonableSpeed& newSpeed);
  //other stuff as unit 
private:
  ReasonableSpeed speed_;
};

With this design SpeedF1 cannot throw, however I need to extra pay an object constructor every time I want to reset the speed.

For class where a limited set of value are reasonable (for example the Months in a calendar) I usually make the constructor private and provide a complete set of static functions. In this case it is impossible, another possibility is implement a null object pattern but I am not sure it is superior to simply throw an exception.

Finally, my question is:

What is the best practise to solve this kind of problem?

Answer 1

First off, don’t overestimate the cost of the extra constructor. In fact, this cost should be exactly the cost of initialising a double plus the cost for the validity check. In other words, it is likely equal to using a raw double.

Secondly, lose the setter. Setters – and, to a lesser degree, getters – are almost always anti-patterns. If you need to set a new (maximum) speed, chances are you actually want a new car.

Now, about the actual problem: a throwing constructor is completely fine in principle. Don’t write convoluted code to avoid such a construct.

On the other hand, I also like the idea of self-checking types. This makes the best use of C++’ type system and I’m all in favour of that.

Both alternatives have their advantages. Which one is best really depends on the exact situation. In general, I try to exploit the type system and static type checking as much as possible. In your case, this would mean having an extra type for the speed.

Answer 2

I strongly vote for the second option. This is only my personal opinion without a lot of academic backing. My experience is that setting up a "pure" system that operates on only valid data makes your code a lot cleaner. This can be achieved by using your second approach which ensures that only valid data enters the system.

If your system grows, you may find that ReasonableSpeed gets used in a lot of places (use your discretion, but chances are things actually get reused quite a lot). The second approach will save you a lot of error checking codes in the long term.

Answer 3

If only one class inherits from ReasonableSpeed then it seems a bit of an overkill.

If many classes inherit from, or use ReasonableSpeed, then it's smart.

Answer 4

Both of your designs yield the same result when an invalid value is used as speed, i.e. they both throw an exception. Applying Occam's razor principle, or Unix Rule of Rarsimony:

Rule of Parsimony: Write a big program only when it is clear by demonstration that nothing else will do.

‘Big’ here has the sense both of large in volume of code and of internal complexity. Allowing programs to get large hurts maintainability. Because people are reluctant to throw away the visible product of lots of work, large programs invite overinvestment in approaches that are failed or suboptimal.

you may like to pick the first simpler approach. Unless you'd like to reuse ReasonableSpeed class.

Answer 5

I would recommend doing this instead:

class SpeedF1
{
public:
  explicit SpeedF1(double maxSpeed);
  double getSpeed() const;
  void accelerate();
  void decelerate();
protected:
  void setSpeed(double speed);
  //other stuff as unit 
private:
  double maxSpeed_;
  double curSpeed_;
};

SpeedF1::SpeedF1(double maxSpeed) maxSpeed_(maxSpeed), curSpeed_(0.0) { }

double SpeedF1::getSpeed() const { return curSpeed_; }

void SpeedF1::setSpeed(double speed) {
    if(speed < 0.0) speed = 0.0;
    if(speed > maxSpeed_) speed = maxSpeed_;
    curSpeed = speed;
}

void SpeedF1::accelerate() {
    setSpeed(curSpeed_ + SOME_CONSTANT_VELOCITY);
}


void SpeedF1::decelerate() {
    setSpeed(curSpeed_ - SOME_CONSTANT_VELOCITY);
}