From 8560a55a5ddb67d6f6004e7d248b2bafc64b6c2e Mon Sep 17 00:00:00 2001
From: Peter Stace <peterstace@gmail.com>
Date: Tue, 12 Oct 2021 05:39:36 +1100
Subject: [PATCH] Add `MinMaxXYs` method to the `Envelope` type

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.
---
 geom/alg_point_on_surface.go |  5 ++---
 geom/type_envelope.go        | 11 +++++++++++
 geom/type_envelope_test.go   | 13 +++++++++++++
 3 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/geom/alg_point_on_surface.go b/geom/alg_point_on_surface.go
index 2efe51b5..9c0f1835 100644
--- a/geom/alg_point_on_surface.go
+++ b/geom/alg_point_on_surface.go
@@ -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
 	}
 
diff --git a/geom/type_envelope.go b/geom/type_envelope.go
index 80275440..13110aa2 100644
--- a/geom/type_envelope.go
+++ b/geom/type_envelope.go
@@ -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.
diff --git a/geom/type_envelope_test.go b/geom/type_envelope_test.go
index dce39743..64ca934f 100644
--- a/geom/type_envelope_test.go
+++ b/geom/type_envelope_test.go
@@ -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)
 			})