gnuastro-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[task #13658] Work on concave polygons too


From: Mohammad Akhlaghi
Subject: [task #13658] Work on concave polygons too
Date: Wed, 8 Apr 2020 10:01:40 -0400 (EDT)
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:74.0) Gecko/20100101 Firefox/74.0

Follow-up Comment #44, task #13658 (project gnuastro):

Great! Thanks Sachin. A few small points remain, I look forward to merging it
afterwards:

* The commit isn't on the most recent commit of the master branch. You can
fetch the most recent commit(s) on the master branch and then rebase over
them.

* The static structures and functions that are only defined in `polygon.c'
(and not `polygon.h') are only relevant during the building of the library.
They aren't accessible for a user of Gnuastro once it is installed. So they
don't need documentation in the book. They should only be documented in the
actual source as comments.

* In the documentation of `gal_polygon_vertices_sort', it would be really
helpful if you explain the methodology of your algorithm a little. In
particular note that for concave polygons there is no unique sorting and that
people should beware (possibly using the polygon-type function to make sure if
it is a convex or concave polygon afterwards). Also, do this when you call
this function in the Crop program's `onecrop.c': put a check after the sorting
and if the polygon is convex, print a warning, with `error(0,0,"WARNING:
...."')  and let the user know that the sorted convex polygon may not be what
they wanted.

* You can set `struct point' to be static in `polygon.c'.

* I noticed multiple return statements in `polygon_compareA' and
`polygon_compareB' that can be made more readable with the `?:' structure
;-).

* Finally, two extra curly-brace blocks (after the `for's) can be removed in
the final set of loops in `gal_polygon_vertices_sort'. Extra curly braces can
be buggy and will always confuse the reader on why you used them? For example
was there something you wanted to change but forgot?

    _______________________________________________________

Reply to this item at:

  <https://savannah.gnu.org/task/?13658>

_______________________________________________
  Message sent via Savannah
  https://savannah.gnu.org/




reply via email to

[Prev in Thread] Current Thread [Next in Thread]