-
Notifications
You must be signed in to change notification settings - Fork 184
Support for geospatial types - point, line, box, polygon etc #306
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome work, thanks a lot. Especially the refactoring towards the Geometry codec was overdue.
I left a few minor comments. Care to have a look?
*/ | ||
public final class Line { | ||
|
||
private final double a; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should add a bit of Javadoc to the fields.
this.c = c; | ||
} | ||
|
||
public static Line of(double a, double b, double c) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it make sense to add factory methods accepting two points or even x1/y1, x2/y2?
@mp911de thanks for the review. I updated the |
Add javadoc to factory methods. Add since tags. Use consistently String.format(…) for toString. Fix Path equals and hashCode. Add license headers. Introduce TokenStream to avoid repeated Double.parse(…) calls. Fix binary representation of Path.open as in Postgres the protocol transmits of the Path is closed so we need to negate the flag. [closes #306][#282]
Thank you for your contribution. That's merged, polished, and backported now. |
Resolves #282