isocpp / isocpp/CppCoreGuidelines
F.3 Example code has too many issues
@BenKeeping is already working on this.
Since Oct 11, 2018.
- Dominant language
- CSS
- Stars
- 45.3k
- Forks
- 5.6k
- PR merge metrics
- No merged PRs in 30d
Description
I know this code is supposed to need refactoring, but I don't think it's the best example to illustrate this point. We tried it in a job interview context and everyone got confused!
First problem - if flag1 = flag2 = 0, intermediate remains undefined and the function returns
finalize(undefined, 0). So at least make the correction
double intermediate = 0.;
2nd problem - when flag1 = -1, its value is changed to 1 - thus the if on case label 1 can never be true.
Therefore the comment in the refactored code about "handling flag1 = -flag1" is meaningless, and flag1 doesn't need to be passed to func1_tau. Since flag1 is received by value, there is no need to refer to it again at all.
3rd problem - return 0 at the end of the refactored code is wrong (unless this is what finalize returns!)
Here is what I believe is a correct refactoring of the original (with the sole change of initialising intermediate mentioned above). But I still don't think this is a good example, hence issue rather than pull request.
double complete(double intermediate, int flag)
{
if (abs(flag) > 10)
intermediate = func2(intermediate);
if (flag / 10 == 2)
return finalize(intermediate, 13.1);
return finalize(intermediate, 0.);
}
double func1_muon(double val, int flag) {
double intermediate = func1(val);
if (flag % 2)
intermediate = sqrt(intermediate);
return complete(intermediate, flag);
}
double func1_tau(double val, int flag) {
double intermediate = func1(-val);
if (flag % 2)
intermediate = sqrt(-intermediate);
return complete(intermediate, flag);
}
double simple_func(double val, int flag1, int flag2)
// simple_func: takes a value and calculates the expected ASIC output,
// given the two mode flags.
{
if (flag1 > 0)
return func1_muon(val, flag2);
if (flag1 == -1)
return func1_tau(-val, flag2);
return finalize(0., 0.);
}
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Assessment
This issue has not been assessed yet.