[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/
- [task #13658] Work on concave polygons too, Sachin Kumar Singh, 2020/04/02
- [task #13658] Work on concave polygons too, Sachin Kumar Singh, 2020/04/02
- [task #13658] Work on concave polygons too, Mohammad Akhlaghi, 2020/04/03
- [task #13658] Work on concave polygons too, Sachin Kumar Singh, 2020/04/04
- [task #13658] Work on concave polygons too, Mohammad Akhlaghi, 2020/04/05
- [task #13658] Work on concave polygons too, Sachin Kumar Singh, 2020/04/06
- [task #13658] Work on concave polygons too, Mohammad Akhlaghi, 2020/04/07
- [task #13658] Work on concave polygons too, Mohammad Akhlaghi, 2020/04/07
- [task #13658] Work on concave polygons too, Sachin Kumar Singh, 2020/04/08
- [task #13658] Work on concave polygons too,
Mohammad Akhlaghi <=
- [task #13658] Work on concave polygons too, Sachin Kumar Singh, 2020/04/08
- [task #13658] Work on concave polygons too, Mohammad Akhlaghi, 2020/04/08
- [task #13658] Work on concave polygons too, Sachin Kumar Singh, 2020/04/09
- [task #13658] Work on concave polygons too, Mohammad Akhlaghi, 2020/04/09