Skip to content

Commit

Permalink
Add MinMaxXYs method to the Envelope type
Browse files Browse the repository at this point in the history
The following pattern was frequently employed by users of the library:

	envMin, minOK := env.Min().XY()
	envMax, maxOK := env.Max().XY()
	if !minOK || !maxOK {
		// envelope is empty, special handling
	}
	// finally do something with envMin and envMax

This works, but has some downsides:

- There is a lot of "min"/"max" stutter in the first three lines.

- It feels a little more verbose than it could be.

- It can be easy for subtle bugs to occur if the mins and maxes don't
  all line up (e.g. min is used in place of max in a single place).

- The predicate in the if statement is awkward because `minOK` and
  `maxOK` should always have the same value. It doesn't feel right to
  ignore one of them though due to the asymmetry. ORing them together
  isn't quite correct either, since it might imply that they could be
  different. ANDing them together also implies that they might be
  different too. The problem is that there are 2 sources of truth for
  one logical boolean value.

The new method changes this pattern to:

	envMin, envMax, ok := env.MinMaxXYs()
	if !ok {
		// envelope is empty, special handling
	}
	// finally do something with envMin and envMax

There's still a _tiny_ bit of stutter between `envMin`, `envMax`, and
`MinMaxXYs`, but there's less chance for bugs to slip in. It's one line
shorter, and there are less variables overall, and there is just one
boolean so the if statement is much simpler.
  • Loading branch information
peterstace committed Oct 11, 2021
1 parent d18f93b commit 8560a55
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 3 deletions.
5 changes: 2 additions & 3 deletions geom/alg_point_on_surface.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,8 @@ func pointOnAreaSurface(poly Polygon) (Point, float64) {
}

// Create bisector.
envMin, minOK := env.Min().XY()
envMax, maxOK := env.Max().XY()
if !minOK || !maxOK {
envMin, envMax, ok := env.MinMaxXYs()
if !ok {
return Point{}, 0
}

Expand Down
11 changes: 11 additions & 0 deletions geom/type_envelope.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,17 @@ func (e Envelope) Max() Point {
return e.max().asUncheckedPoint()
}

// MinMaxXYs returns the two XY values in the envelope that contain the minimum
// (first return value) and maximum (second return value) X and Y values in the
// envelope. The third return value is true if and only if the Envelope is
// non-empty and thus the first two return values are populated.
func (e Envelope) MinMaxXYs() (XY, XY, bool) {
if e.IsEmpty() {
return XY{}, XY{}, false
}
return e.min(), e.max(), true
}

// ExtendToIncludeXY returns the smallest envelope that contains all of the
// points in this envelope along with the provided point. It gives an error if
// the XY contains NaN or +/- Infinite coordinates.
Expand Down
13 changes: 13 additions & 0 deletions geom/type_envelope_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,19 @@ func TestEnvelopeAttributes(t *testing.T) {
t.Run("Max", func(t *testing.T) {
expectGeomEqWKT(t, tc.env.Max().AsGeometry(), tc.max)
})
t.Run("MinMaxXYs", func(t *testing.T) {
gotMin, gotMax, gotOK := tc.env.MinMaxXYs()
expectBoolEq(t, gotOK, !tc.isEmpty)
if gotOK {
wantMin, minOK := geomFromWKT(t, tc.min).AsPoint().XY()
expectTrue(t, minOK)
expectXYEq(t, gotMin, wantMin)

wantMax, maxOK := geomFromWKT(t, tc.max).AsPoint().XY()
expectTrue(t, maxOK)
expectXYEq(t, gotMax, wantMax)
}
})
t.Run("AsGeometry", func(t *testing.T) {
expectGeomEqWKT(t, tc.env.AsGeometry(), tc.geom, IgnoreOrder)
})
Expand Down

0 comments on commit 8560a55

Please sign in to comment.