ClaasM / Algorithms

A comprehensive and exhaustive resource for algorithms in all languages and versions.
MIT License
19 stars 7 forks source link

Point in polygon singularity fix #2

Closed gerritnowald closed 2 years ago

gerritnowald commented 2 years ago

Hi! I stumbled on this algorithm on your amazing website and implemented it in my polygon module (a collection of functions to calculate properties of arbitrary polygons, you can find it in my repositories). While adapting and understanding it I changed some things which I want to propose to you.

The most important thing is that there was a singularity in the original condition. This also has a geometrical meaning, namely that the edge to be tested is parallel to the test line, i.e. the x-axis. So I catched this singularity, because it makes the following checks useless. Then I separated the condition into its parts. I think this makes the algorithm easier to understand and removes unnecessary calculations (later conditions only have to be checked if the previous apply). Finally I replaced the while loop with a for loop, since the number of loops are known in advance. This removes the need to define and update the counter variable.

I kept the original code wherever possible and also kept the original comments. Overall I think you did a very clever implementation of the algorithm! I would be happy if these changes suit you and love to get in touch with you.

Best Gerrit

gerritnowald commented 2 years ago

Hi again, I made another commit removing the binary and reversing my changes to gitignore. Looks good to me now, if you want you can approve it now :)

ClaasM commented 2 years ago

LGTM, thanks for the contribution :)