go-hep / hep

hep is the mono repository holding all of go-hep.org/x/hep packages and tools
https://go-hep.org
BSD 3-Clause "New" or "Revised" License
230 stars 35 forks source link

fastjet/internal/delaunay: consider replacing Triangle.{A,B,C} with Triangle.Points #79

Open sbinet opened 7 years ago

sbinet commented 7 years ago

Paraphrasing @vladimir-ch:

Consider replacing A, B, C *Point: https://github.com/go-hep/hep/blob/b0c08641fd2eed633fca25b79dd60b72c56bc828/fastjet/internal/delaunay/triangle.go#L19

// Triangle is a set of three points that make up a triangle.
// It stores hierarchical information to find triangles.
type Triangle struct {
    // children are triangles that lead to the removal of this triangle.
    // When a point is inserted the triangle that contains the point is found by going down the hierarchical tree.
    // The tree's root is the root triangle and the children slice contains the children triangles.
    children []*Triangle
    // A,B,C are the CCW-oriented points that make up the triangle
    A, B, C *Point
    // isInTriangulation holds whether a triangle is part of the triangulation
    isInTriangulation bool
}

with a simple field Points [3]*Point. This would simplify the code at various places. For example in findNearest.

Bastiantheone commented 7 years ago

I agree it would definitively simplify the code, but I think using A,B and C makes it more user friendly.

Bastiantheone commented 7 years ago

Do you think I should change it anyways? @sbinet

sbinet commented 7 years ago

I am agnostic for the moment. I'll play with the proposed change during the week-end and see whether I like what I see :)

Bastiantheone commented 7 years ago

Sounds good :)