-
-
Notifications
You must be signed in to change notification settings - Fork 38
Proposed interface for triangulation #23
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
Add the description for an interface for triangulation.
@olt would love to get your input on this as well. |
planar/triangulate/triangulate.go
Outdated
|
||
type Interface interface { | ||
// SetPoints sets the nodes to be used in the triangulation | ||
SetPoints(pts []geom.Point, data []interface{}) |
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.
what is data
?
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.
Should SetPoints overwrite all existing points? If not, then I would call it AddPoints
.
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.
What about error handling? Could there be any conditions where the added points are invalid? When should the triangulation happen? I think either SetPoints or Triangles should be allowed to return an error (if not both).
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.
Got point.
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.
Data is arbitary data that maybe attached to the vertex.
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.
How can we access the data? Via geom.Triangle? Is the same data attached to all vertices that are build from pts?
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.
This is a good point. Maybe Triangles should return a [][3]interface{} for the data associated with each triangle? The more I think about this, the more I think we should drop the metadata, and allow the user to manage that outside of this interface.
planar/triangulate/triangulate.go
Outdated
Triangles() []geom.Triangle | ||
} | ||
|
||
type Constrainer interface { |
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.
This should probably come with a bit more description. Maybe reference Constrained Delaunay triangulation?
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.
If AddContraints
is always tied to a Triangulator
, then I would call this interface ContrainedTriangulator
and embed the Triangulator
interface:
type ContrainedTriangulator interface {
Triangulator
AddConstraints(constraints ...geom.Line) error
}
planar/triangulate/triangulate.go
Outdated
|
||
import "github.com/go-spatial/geom" | ||
|
||
type Interface interface { |
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.
How about Triangulator
?
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.
Sure!
planar/triangulate/triangulate.go
Outdated
|
||
type Interface interface { | ||
// SetPoints sets the nodes to be used in the triangulation | ||
SetPoints(pts []geom.Point, data []interface{}) |
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.
What about error handling? Could there be any conditions where the added points are invalid? When should the triangulation happen? I think either SetPoints or Triangles should be allowed to return an error (if not both).
planar/triangulate/triangulate.go
Outdated
Triangles() []geom.Triangle | ||
} | ||
|
||
type Constrainer interface { |
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.
If AddContraints
is always tied to a Triangulator
, then I would call this interface ContrainedTriangulator
and embed the Triangulator
interface:
type ContrainedTriangulator interface {
Triangulator
AddConstraints(constraints ...geom.Line) error
}
5f8df51
to
0cdd71d
Compare
Updated interfaces according to feedback.
0cdd71d
to
fcb58ca
Compare
closed via #88 |
This pull request is here to discuss the Proposed triangulation interface.
Add the description for an interface for triangulation.