Question

I want to have definition for pow2C as follows. But my compiler shows it (__tmpDouble__) is already defined. Please help me define it.

#ifndef pow2C
double __tmpDouble__;
#define pow2C(a) ((__tmpDouble__=(a))*(__tmpDouble__))
#endif

.
.
.

inline double calculateDistance(double *x, double *y, int NDims) 
{
    double d = 0;
    for (int i = 0; i < NDims; i++)
    // it is faster to calculate the difference once
       d += pow2C(x[i] - y[i]);
    return sqrt(d);
}
Was it helpful?

Solution 3

I want to calculate x[i]-y[i] only once and save it in a variable and use it.

You already do only calculate it once. This is all pointless.

If you still insist, then the error you name does not exist in this code.

However, you do have a problem with the content of the macro, which both reads from and writes to a single variable within an expression. The result is undefined:

#include <iostream>
#include <cmath>
#include <array>

#ifndef pow2C
double __tmpDouble__;
#define pow2C(a) ((__tmpDouble__=(a))*(__tmpDouble__))
#endif


inline double calculateDistance(double *x, double *y, int NDims) 
{
    double d = 0;
    for (int i = 0; i < NDims; i++)
    // it is faster to calculate the difference once
       d += pow2C(x[i] - y[i]);
    return sqrt(d);
}

int main()
{
    const size_t NUM_DIMS = 3;
    std::array<double, NUM_DIMS> x{{5.0, 6.0, 7.0}};
    std::array<double, NUM_DIMS> y{{1.2, 1.5, 9.0}};

    std::cout << "Distance between x and y is: " << calculateDistance(&x[0], &y[0], NUM_DIMS) << '\n';
}

// g++-4.8 -std=c++11 -Wall -pedantic -pthread main.cpp && ./a.out
// main.cpp: In function 'double calculateDistance(double*, double*, int)':
// main.cpp:16:31: warning: operation on '__tmpDouble__' may be undefined [-Wsequence-point]
//         d += pow2C(x[i] - y[i]);
//                                ^
// Distance between x and y is: 6.22013

So, really, we're back to don't do this.


If you're still desperate to avoid evaluating x[i] - y[i] twice:

inline double calculateDistance(double* x, double* y, int NDims) 
{
    double d = 0;
    for (int i = 0; i < NDims; i++) {
       const double diff = x[i] - y[i];
       d += diff * diff;
    }

    return sqrt(d);
}

You can pass off the multiplication work to another inline utility function if you like, but there is no need for a macro here.

OTHER TIPS

If you insist on writing a macro, use a different name for the variable. But your macro is very terrible. First, it isn't thread safe with regard to concurrent calls, since the threads would access the same temporary variable. Also, most macros are very evil in general (there are some exceptions though).

However, the same (also performance-wise) can be achieved much easier (even with support for float and other types with operator* with a function template:

template <typename T>
T square(const T &v) {
    return v * v;
}

If you want a macro, then #define pow2C square (sarcasm sign).

That's a terrible way to define a macro! You should never introduce variables as part of the calculation. Instead, try this:

#define pow2C(a) ((a) * (a))

However, macros are terrible for this sort of computation as they can be inefficient and cause unwanted side effects. For example, if you code it would expand to:

 (x[i] - y[i]) * (x[i] - y[i])

Where you've done the subtraction twice!

Instead, use C++ templates:

template<class T>
T pow2C(const T &value)
{
  return value * value;
}

Now you'll only do the subtraction calculation once!

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top