Fix compiler warnings from MagfieldCoils

richeldichel commented 1 year ago

As reported in issue #84 there appear many compiler warnings when building MagfieldCoils.cxx (mostly [-Wmisleading-indentation] and [-Wunused-variable].

This pull request removes the warnings and further applies .clang-format to MagfieldCoils.cxx and MagfieldCoils.h

richeldichel commented 1 year ago

[ ] It is quite difficult to understand which variables are member variables and which are local. I would suggest refactoring member variables to the common naming scheme in Kassiopeia, being fVariableName.

[ ] Is it correct to return "true" in the functions if nmax is 0? Shouldn't that raise an error? Also, I think it is redundant to have an inner counting variable like int i that is followed by an outer counting variable int nmax, but that maybe doesn't matter that much.

[ ] Can one remove https://github.com/KATRIN-Experiment/Kassiopeia/blob/4cca8e75612c507b3dd182b08443694dc4344761/KEMField/Source/ExternalFields/MagfieldCoils/CMakeLists.txt#L42 now?

Thanks for the remarks @2xB. I also see these issues. Unfortunately I also do not have the clearest overview on this code. I would propose, because I feel like these issues need a little more time than I can offer at the moment, to go ahead with this merge and put the issues onto the issue tracker.

We can, however, already remove the compile option that you mentioned!

2xB commented 6 months ago

Nice! I did the last remaining open point according to an answer Ferenc wrote back then (nmax <= 1 can't be used, nmax=500 is recommended, so it now throws a corresponding error in case of nmax <= 1). Since I was a bit fast committing using the web IDE, there are two amends as separate commits, which I have to apologize for.

richeldichel commented 5 months ago

Nice! I did the last remaining open point according to an answer Ferenc wrote back then (nmax <= 1 can't be used, nmax=500 is recommended, so it now throws a corresponding error in case of nmax <= 1). Since I was a bit fast committing using the web IDE, there are two amends as separate commits, which I have to apologize for.

Thanks! I squashed your two additional commits so that this pull request can now also be finally merged.

KATRIN-Experiment / Kassiopeia

Fix compiler warnings from MagfieldCoils #86