APR 'findHelix' method indices are overloaded, needs comments and subdivision

[brownd1978]

findHelix overloads integers used both as indices in a triple-nested loop and as flags for various conditions. The logical conditions should be split out as explicit flags. The loops should be refactored into functions, and the intent of this code should be described in comments.

[gianipez]

@matthewstortini was this solved?

[matthewstortini]

@matthewstortini was this solved?

@gianipez Not yet. I wanted to let Ethan finish the changes we are implementing first. Perhaps he can cancel his PR and I can address this problem in his area so that all of the changes can be in one PR.

I am not sure that the module needs to be refactored into functions as much as it may seem when one quickly scrolls through it. All of the logical steps are factorized now. One loop looks ugly mostly because there is a lot of debugging folded in. This debugging was for early development and uses MC info. It will all be removed in the short future.

[kutschke]

I would like to keep these as separate PRs so that they are reviewable.

Will this work: you fix the problem in this issue, make a PR that is reviewed and merged. Then Ethan can merge/rebase his PR and continue his work. If you do this, do you expect that Ethan will need to resolve extensive merge conflicts?

And to remind you. There are two ways to break out of a doubly nested loop. The strongly preferred way is to encapsulate the double loop in a function and just return from it. If there is a good reason to not do that, then use a goto.

[kutschke]

@matthewstortini and @gianipez is there anything you would like clarified in my request?

[matthewstortini]

@kutschke I understand, no need for further clarification. Ethan's PR should be very simple and go through fairly quickly (I hope in a day or two). I will not be able to get to this issue until later in the week. Thus, I think Ethan's PR will be through before I make my own anyway.