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?
1 Answer 1
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).