Is it better to store class constants in data members or in methods?

Question

I recently wrote a class that renders B-spline curves. These curves are defined by a number of control points. Originally, I had intended to use eight control points, so I added a constant to the class, like so:

class Curve
{
   public:
      static const int CONTROL_POINT_COUNT = 8;
};

Now I want to extend this class to allow an arbitrary amount of control points. So I want to change this to:

class Curve
{
   public:
      int getControlPointCount() {return _controlPointCount;}
};

The question is whether it isn't better to store constants in methods to begin with, to facilitate adaptability. In other words, isn't it better to have started thus:

class Curve
{
   public:
      int getControlPointCount() {return 8;}
};

The advantage of this is that I could have just changed one symbol in the method in question, instead of moving around constants etc.

Is this a good practice or a bad one?

Answer 1

To better answer your question, one should also know how the controlPointCount variable is set. Is it set outside from your class? In this case, you should also define a setter. Or the Curve class is the sole responsible for setting it? Is it set only on compile time or also on runtime.

Anyway, avoid a magic number even in this form:

int getControlPointCount() {return 8;}

This is better:

int getControlPointCount() {return CONTROL_POINT_COUNT;}

A method has the advantage that you can modify the internal implementation (use a constant value, read from a configuration file, alter the value dynamically), without affecting the external of the class.

Answer 2

int getControlPointCount() {return _controlPointCount;}

This is an accessor. Swapping a const static for an accessor is not really a gain as litb has pointed out. What you really need to future-proof is probably a pair of accessor and mutator.

int getControlPointCount() {return _controlPointCount;} // accessor

I'd also throw in a design-const for the accessor and make it:

int getControlPointCount() const {return _controlPointCount;} // accessor

and the corresponding:

void setControlPointCount(int cpc) { _controlPointCount = cpc;} //mutator

Now, the big difference with a static object is that the control-point count is no longer a class-level attribute but an instance level one. This is a design change. Do you want it this way?

Nit: Your class level static count is public and hence does not need an accessor.

Answer 3

In general, all your data should be private and accessed via getters and setters. Otherwise you violate encapsulation. Namely, if you expose the underlying data you lock yourself and your class into a particular representation of that underlying data.

In this specific case I would have done the something like the following I think:

class Curve
{

   protected:

      int getControlPointCount() {return _controlPointCount;}
      int setControlPointCount(int c) { _controlPointCount = c; }

   private:

      static int _controlPointCount = 0;
};

Answer 4

class Curve
{   
    private:
        int _controlPointCount;

        void setControlPointCount(int cpc_arg)
        {
            _controlPointCount = cpc_arg;
        }

    public:      
        curve()
        {
            _controlPointCount = 8;
        }

        int getControlPointCount() const
        {
            return _controlPointCount;
        }
};

I will create a code like this, with set function in private, so that no body can play with control point count, until we move to the next phase of development..where we update start to update the control point count at runtime. at that time, we can move this set method from private to public scope.

Answer 5

Typically I favour maintaining as few couplings manually as possible.

The number of control points in the curve is, well, the number of control points in the curve. It's not an independent variable that can be set at will.

So I usually would expose a const standard container reference:

class Curve
{   
    private:
        std::vector<Point>& _controlPoints;

    public:      
        Curve ( const std::vector<Point>& controlPoints) : _controlPoints(controlPoints)
        {
        }

        const std::vector<Point>& getControlPoints ()
        {
            return _controlPoints;
        }
};

And if you want to know how many control points, then use curve.getControlPoints().size(). I'd suspect that in most of the use cases you'd want the points as well as the count anyway, and by exposing a standard container you can use the standard library's iterator idioms and built-in algorithms, rather getting the count and calling a function like getControlPointWithIndex in a loop.

If there really is nothing else in the curve class, I might even go as far as:

typedef std::vector<Point> Curve;

(often a curve won't render itself, as a renderer class can have details about the rendering pipeline, leaving a curve as purely the geometric artifact)

Answer 6

While understanding the question, I have a number of conceptual problems with the example:

• What is the return value for getControlPointCount() when the number of control points is not limited?
  • Is it MAXINT?
  • Is it the current number of control points on the curve (thus breaking the logic that says that this is the largest possible number of points?)
• What happens when you actually attempt to create a curve with MAXINT points? You will run out of memory eventually.

The interface itself seems problematic to me. Like other standard collection classes, the class should have encapsulated its limitation on number of points, and its AddControlPoint() should have returned an error if a limitation on size, memory, or any other violation has occurred.

As for the specific answer, I agree with kgiannakakis: a member function allows more flexibility.

Answer 7

I tend to use configuration + constant (default value) for all 'stable' values through the execution of the program. With plain constants for values that cannot change (360 degrees -> 2 pi radians, 60 seconds -> 1 minute) or whose change would break the running code (minimum/maximum values for algorithms that make them unstable).

You are dealing with some different design issues. First you must know whether the number of control points is a class or instance level value. Then whether it is a constant at any of the two levels.

If all curves must share the same number of control points in your application then it is a class level (static) value. If different curves can have different number of control points then it is not a class level value, but rather a instance level one.

In this case, if the number of control points will be constant during the whole life of the curve then it is a instance level constant, if it can change then it is not constant at this level either.

// Assuming that different curves can have different 
// number of control points, but that the value cannot 
// change dynamically for a curve.
class Curve
{
public:
   explicit Curve( int control_points )
      : control_points_( control_points )
   {}
   // ...
private:
   const int control_points_;
};

namespace constant
{
   const int spline_control_points = 8;
}
class Config
{
public:
   Config();
   void readFile( std::string const & file );

   // returns the configured value for SplineControlPoints or
   // constant::spline_control_points if the option does not 
   // appear in config.
   int getSplineControlPoints() const;
};

int main()
{
   Config config;
   config.readFile( "~/.configuration" ); // read config

   Curve c( config.getSplineControlPoints() );
}

Answer 8

For integral type I'm usualy using:

class Curve
{
   public:
      enum 
      {
          CONTROL_POINT_COUNT = 8
      };
};

If constant doesn't need for any entities except class implementation I declare constants in *.cpp file.

namespace
{
const int CONTROL_POINT_COUNT = 8;
}

Answer 9

Constants in general should not be defined inside methods. The example you're choosing has two unique features. First, it's a getter; second, the type being returned is an int. But the point of defining constants is to use them more than once, and to be able to refer to them in a convenient way. Typing "8" as opposed to "controlPointCount" may save you time, and may not seem to incur a maintenance cost, but this won't typically be true if you always define constants inside methods.