What is the best practice when coding math class/functions ?

Question

Introductory note : I voluntarily chose a wide subject. You know that quote about learning a cat to fish, that's it. I don't need an answer to my question, I need an explanation and advice. I know you guys are good at this ;)

---

Hi guys,

I'm currently implementing some algorithms into an existing program. Long story short, I created a new class, "Adder". An Adder is a member of another class representing the physical object actually doing the calculus , which calls adder.calc() with its parameters (merely a list of objects to do the maths on).

To do these maths, I need some parameters, which do not exist outside of the class (but can be set, see below). They're neither config parameters nor members of other classes. These parameters are D1 and D2, distances, and three arrays of fixed size : alpha, beta, delta.

I know some of you are more comfortable reading code than reading text so here you go :

class Adder
{
public:

  Adder();
  virtual Adder::~Adder();

void set( float d1, float d2 );
  void set( float d1, float d2, int alpha[N_MAX], int beta[N_MAX], int delta[N_MAX] );

  // Snipped prototypes
  float calc( List& ... );
  // ...

  inline float get_d1() { return d1_ ;};
  inline float get_d2() { return d2_ ;};

private:

  float d1_;
  float d2_;

  int alpha_[N_MAX]; // A #define N_MAX is done elsewhere
  int beta_[N_MAX]; 
  int delta_[N_MAX]; 
};

Since this object is used as a member of another class, it is declared in a *.h :

private:
  Adder adder_;

By doing that, I couldn't initialize the arrays (alpha/beta/delta) directly in the constructor ( int T[3] = { 1, 2, 3 }; ), without having to iterate throughout the three arrays. I thought of putting them in static const, but I don't think that's the proper way of solving such problems.

My second guess was to use the constructor to initialize the arrays

Adder::Adder()
{
    int alpha[N_MAX] = { 0, -60, -120, 180, 120,  60 };
    int beta[N_MAX] = { 0,   0,    0,   0,   0,   0 };
    int delta[N_MAX]  = { 0,   0,  180, 180, 180,   0 };

    set( 2.5, 0, alpha, beta, delta );
}

void Adder::set( float d1, float d2 ) {
    if (d1 > 0)
        d1_ = d1;

    if (d2 > 0)
        d2_ = d2;
}

void Adder::set( float d1, float d2, int alpha[N_MAX], int beta[N_MAX], int delta[N_MAX] ) {
    set( d1, d2 );

    for (int i = 0; i < N_MAX; ++i) {
        alpha_[i] = alpha[i];
        beta_[i] = beta[i];
        delta_[i] = delta[i];
    }
}

My question is : Would it be better to use another function - init() - which would initialize arrays ? Or is there a better way of doing that ?

My bonus question is : Did you see some mistakes or bad practice along the way ?

Answer 1

Okay. I would:

1. Set the arrays to a known state by using memset to clear all values to 0 (or some other value) within the constructor before they are used.
2. Change the constructor to allow the passing of array pointers that can be used to initialise the arrays to some other values.
3. Retain the Set function that you have to change the values within the arrays and ariables that you're using.

Answer 2

You have chosen a very wide subject, so here is a broader answer.

• Be aware of your surroundings

Too often I have seen code doing the same thing as elsewhere in the codebase. Make sure that the problem you are trying to solve has not already been solved by your team-mates or predecessors.

• Try not to reinvent the wheel

An extension of my previous point.

While everyone should probably write a linked-list or a string class as an exercise, there is no need to write one for production code. You will have access to MFC, OWL, STL, Boost, whatever. If the wheel has already been invented, use it and get on with coding a solution to the real problem!

• Think about how you are going to test your code

Test Driven Development (TDD) is one way (but not the only way) to ensure that your code is both testable and tested. If you think about testing your code from the beginning, it will be very easy to test. Then test it!

• Write SOLID code

The Wikipedia page explains this far better than I could.

• Ensure your code is readable

Meaningful identifiers are just the beginning. Unnecessary comments can also detract from readability as can long functions, functions with long argument lists (such as your second setter), etcetera. If you have coding standards, stick to them.

• Use const more

My major gripe with C++ is that things aren't const by default! In your example, your getters should be declared const and your setters should have their arguments passed in as const (and const-reference for the arrays).

• Ensure your code is maintainable

Unit tests (as mentioned above) will ensure that the next person to change your code doesn't break your implementation.

• Ensure your library is usable

If you follow Principle of least astonishment and document your library with unit-tests, programmers using it will have fewer issues.

• Refactor mercilessly

An extension of the previous point. Do everything you can to reduce code duplication. Today I witnessed a bug-fix that had to be executed in fifteen separate places (and was only executed in thirteen of these).

Answer 3

1. Don't use virtual functions, unless your design actually requires them.
2. In a class that exists primarily to execute one function, that function is canonically named operator(). I.e. you'd call yours as adder_(params), not adder_.calc(params).
3. If you're initializing three arrays, it's more efficient to use three for-loops. (cache friendly)