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?

asked Jun 3, 2013 at 7:39
\$\endgroup\$

1 Answer 1

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).

answered Jun 5, 2013 at 9:32
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.