2
\$\begingroup\$

I have a function in the C++ program, which does simple computations and writes result to two arrays - result[53] and var1_new[53]. It contains two code blocks:

The first one -

double result[53];

result[0] = ((var1[13] * 1) + (var2[7] * 2)) / var3[0];
result[1] = ((var1[21] * 1) + (var2[7] * 2)) / var3[1];
result[2] = ((var1[12] * 2) + (var2[7] * 3)) / var3[2];
result[3] = ((var1[25] * 2) + (var2[7] * 3)) / var3[3];
//And so on, up to result[52]

And the second one -

double var1_new[53];

for (int x = 0; x < 53; ++x)
{
    var1_new[x] = var1[x];
}

var1_new[13] += result[0];
var1_new[21] += result[1];
var1_new[12] += result[2];
var1_new[25] += result[3];
//And so on, up to var1_new[52]

The blocks above use equal array indexes pattern - 13, 21, 12, 15, etc. The pattern is hardcoded.

This code runs pretty fast, thus I don't want to increase performance, but I'm thinking about readability - the function that contain code blocks is very complex. How can I make it more readable? Is it even possible?

\$\endgroup\$
5
\$\begingroup\$

First, magic constants are bad, you should name them

int const N = 53;
double result[N];

Second, magic patterns are even worse, you should also name them along with documentation comments

// explain where the pattern comes from
int const pattern[] = { 13, 21, 12, 25, /* 49 more constants */ };

Third, manually loop unrolling can sometimes help, but only after you benchmarked

for (int i = 0; i < N; ++i) {
    result[i] = ((var1[pattern[i]] * (i+1)) + (var2[7] * (i+2))) / var3[i];
}

For your second part, rinse and repeat:

double var1_new[N];

Fourth, use the Standard Library for standardized algorithms

#include <algorithm>
std::copy_n(var1, N, var1_new);

Again, rinse and repeat the loop unrolling

for (int i = 0; i < N; ++i) {
    var1_new[pattern[i]] += result[i];
}

Finally, you might want to give your variables meaningful names: var1, var2, var3 do not explain their intended use very well. Think about what they mean in your application (it was unclear from the code fragment alone).

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.