Using #define directives as "part" of code, not at top of file

Question

When looking at c++ source code, I almost always see #define macros at the head of the file, this makes sense in most cases, and I can see why this is best practice, but I recently came upon a situation where it might be better to keep the preprocessor definitions in a function body.

I'm writing a quaternion class, and my code for the function in question looks like this:

Quaternion Quaternion::operator*(const Quaternion& q){
    Quaternion resultQ;

    // These are just macros to make the code easier to read,
    // 1 denotes the quaternion on the LHS of *,
    // 2 denotes the quaternion of the RHS 0f *, e.a. Q1 * Q2.
    // the letter denotes the value of the real number constant part 
    // for each seperate part of the quaterion, e.a. (a+bi+cj+dk)

    #define a1 this->get_real()
    #define a2 q.get_real()
    #define b1 this->get_i()
    #define b2 q.get_i()
    #define c1 this->get_j()
    #define c2 q.get_j()
    #define d1 this->get_k()
    #define d2 q.get_k()

    // This arithemtic is based off the Hamilton product
    // (http://en.wikipedia.org/wiki/Quaternion#Hamilton_product)
    resultQ.set_real(a1*a2 - b1*b2 - c1*c2 - d1*d2);
    resultQ.set_i(a1*b2 + b1*a2 + c1*d2 - d1*c2);
    resultQ.set_j(a1*c2 - b1*d2 + c1*a2 + d1*b2);
    resultQ.set_k(a1*d2 + b1*c2 - c1*b2 + d1*a2);
    return resultQ;
}

I decided to add in the #define because if I substituted in all the macros manually, each line would be too long, and either be cut off or carried over to the next line when read. I could have done the same thing with variables, but I decided that would be an unnecessary overhead, so I used #define because it has no runtime overhead. Is this an acceptable practice? Is there a better way to make what I am doing here readable?

Answer 1

Instead of

#define a1 this->get_real()

write

auto const a1 = get_real();

And just use different names for each value of a quantity that changes.

Yes there are cases where a local #define makes sense. No this is not such a case. In particular, since you've forgotten to #undef the macros they will almost certainly cause inadvertent text substitution in some other code, if this is in a header (as indicated).

---

By, the way, instead of

Quaternion Quaternion::operator*(const Quaternion& q){

I would write

Quaternion Quaternion::operator*(const Quaternion& q) const {

so that also const quaternions can be multiplied.

Answer 2

Macros don't respect scope. Those macros are defined for the rest of the file after they appear. If you have a variable a1 in the next function, it will mess up. You should #undef all the macros at the end of the function.

Better is to create some utility functions somehow. In C++11

auto a1 = [this](){ return this->get_real(); }
...

resultQ.set_real(a1()*a2() ...

Not quite the same as you need the () but maybe good enough for you.

If the values of a1 etc don't change during the calculations, you should use Alf's suggestion.

Answer 3

The other answers already give alternatives to macros. You should definitely follow one such alternative because this use of macros in your code is unnecessary and bad.

But I feel your code needs redesign anyway, after which you'll see that neither macros nor their alternatives are needed.

A quaternion is a generalization of a complex number, and as such, essentially it has a number of data members (four rather than two), a number of constructors, and a number of operators.

You might have a look at the design of std::complex to get ideas. The design of a quaternion need not be much different. In particular, why would you need setters/getters to access data from a member function? These methods are exactly what makes expressions long! (along with the unnecessary use of this).

So, if the data members of a quaternion are a, b, c, d, and if there is a constructor with these four values are arguments, then your operator* should really look like this:

Quaternion Quaternion::operator*(const Quaternion& q) const
{
    return Quaternion{
       a*q.a - b*q.b - c*q.c - d*q.d,
       a*q.b + b*q.a + c*q.d - d*q.c,
       a*q.c - b*q.d + c*q.a + d*q.b,
       a*q.d + b*q.c - c*q.b + d*q.a
    };
}

No need for macros, helper functions, or intermediate variables.