refactor to simplify and avoid slicing risk

[atsju]

This is same as #218 but full refactor to simplify code at maximum.
Chose pull request that you prefer. #218 is reviewed easier because no code has been displaced. This one is easier to maintain in the futur (unless we want to support ellipses).

• deleted gplus.cpp and gplus.h
• deleted boundary.h
• deleted unused functions in circleOutline.h
• reordered functions to make sense and have same order in .cpp and .h
• linted code using clang-format (adds and removes spaces, reorders #include, ...)
• added comments in doxygen format

If you are interested into clang-format you can look at the effects on https://clang-format-configurator.site/
To actually use it, most modern IDE support it natively.
I did not commit the .clang-format file to the repo so that complete files are not linted automatically whenever you change 1 character in a file. I read it's natively supported in QT Creator.

[gr5]

The code differences seems pretty massive in the diff so I didn't read much of it. Especially since Dale already approved. I did build it and test it and elliptical and circular outlines both still seem to work fine.

[atsju]

@gr5 we do not support elliptical outlines. Do we ?
From my understanding. There were empty classes for elliptical but not implemented and when you upgraded .oln files to json you added the elliptical information into file to be "future ready". I did not remove the information from json file but we still do not support elliptical outline. And with changes from this PR: we probably never will.

So which one do we merge ?

• ensure there is no slicing risk on inherited classes #218 : minimal code changes, more ready for future update to add elliptical outlines
• refactor to simplify and avoid slicing risk #220 : heavily simplified code, linted and added comments (details in PR description)

[githubdoe]

We do support elliptical outlines I think because we support measuring diagonal mirrors which can be elliptical. Perhaps they don't use the elliptical outline itself but I think they do.

See what happens when you set the config to be an ellipse and then drag the horizontal axis or vertical axis to outline something.

[githubdoe]

I just checked and we do support elliptical outline when testing an ellipse. I think George wrote the json code for that. I think that was the first task I ever assigned to him. That is saving outlines in json. IIRC.

[atsju]

I see. Thank you @githubdoe.
Now I'm sure I didn't broke anything. Probably what I removed was a first architecture try that has then be reworked to use something different.
I will need to look at the architecture and understand how it's done before we merge this.

[githubdoe]

Note to George. Just because I approve something does not mean I checked it very closely. Like I have said recently I don't have a lot of time. In the last case I just decided that since you will check things more than I do it would be ok to approve it since I did not see anything that stood out glaringly. So we sort of created a loop of I don't need to because the other said it was ok. Oh, Oh,.

[gr5]

Yesterday, I was specifically nervous that Julien broke elliptical outlines but like I posted yesterday, I did build this version (this PR) and I did test the elliptical outline feature and it worked fine. I didn't think of saving the wavefront and reading it back in so I'll do that now. I'll also test that mirror config can be saved and loaded and elliptical still works.

But I suspect it will be fine.

In general, I don't think every line of code needs a detailed code review. Julien has been quite reliable in the past. Usually I look at the code carefully but this time I just tested it by running the program. Because the code changes are only related to outlines I think it's easy enough to do a good test by running the code.

Julien did however make a mistake in a zernike formula in one of his PRs and that I looked at very very carefully because that would not be easily caught or noticed by users. Julien caught his own mistake. I would have caught it as well as those were very critical lines of code.

[gr5]

And Dale, if I review and approve the code before you do, you can bet I looked at it extra carefully. If you approve it before I do then I don't look as carefully. You could I suppose mention in the approval comments specifically that you didn't check every line of code.

The last few reviews I did look at every line of code. But this one has a lot of changes.

[gr5]

I did more testing and still approve this pull. I did find two bugs but they are in 7.4.0 also.

First what works fine:
process elliptical igrams. works fine.
Save elliptical wavefront and load it. works fine.
saving elliptical outline to and reading back from OLN file works fine.

bugs:

If you check the ellipse checkbox in mirror config, save the config and load the config, the elliptical checkbox is unchecked. I'll create an issue. This is in 7.4.0 also. The information is stored in the json file but something goes wrong when read back. I may have introduced this bug. Not sure. From json file:

Profile plot displays elliptical mirror as though it's a circle. Distances to edge of mirror shown on scale are wrong (or width of contour is wrong - should fix one or the other) Maybe this is fine but the dotted line that appears in the contour plot is not centered over the mirror and looks very strange.

[githubdoe]

I am a terrible code reviewer and editor. I see what I want to see and not what is really there most of the time. So usually even when I try to be careful you can not trust I got it right. These days I seem to have trouble with understanding gits diff some times as well. What I try to do is understand why the change happened and if the general structure and a few of the lines look like it should correct the problem. But I rarely examine every line.

[atsju]

OK I had a look and circleOutline draws elipses. There are many parts of igramArea which call ellipse things.
We will probably never need the inheritance I removed because of how it's coded.
I will merge this now