simplefeatures icon indicating copy to clipboard operation
simplefeatures copied to clipboard

Function for NewCoordinates

Open akhenakh opened this issue 1 year ago • 2 comments

I run into a stupid bug because specifying Z in a NewPoint(geom.Coordinates{}) results in it defaulting to Type: geom.DimXY, it makes sense.

package main

import (
	"fmt"

	"github.com/peterstace/simplefeatures/geom"
)

func main() {
	p := geom.NewPoint(geom.Coordinates{
		XY: geom.XY{
			X: 10,
			Y: 11,
		},
		Z: 400,
	}).AsGeometry()

	fmt.Println(p.DumpCoordinates())
	fmt.Println(p.AsText())
}
{0 [10 11]}
POINT(10 11)

In the rest of the API, though, helper functions helps to remind you to specify the dimension like geom.NewSequence(seq, geom.DimXY).

I did not find any NewCoordinates() ?

akhenakh avatar Mar 12 '24 18:03 akhenakh

Yeah I see what you mean, thank you for reporting your experience. I think there are a few different options to make this sort of bug less likely:

Option 1

Add a geom.NewCoordinates function that takes a geom.CoordinatesType. We'd also have to make it accept the right number of float64 arguments. So maybe the function signature would be like geom.NewCoordinates(geom.CoordinatesType, xyzm ...float64). Examples of use:

  • geom.NewPoint(geom.NewCoordinates(geom.DimXY, 10, 11))
  • geom.NewPoint(geom.NewCoordinates(geom.DimXYZ, 10, 11, 400))

It might be best if it panics if the geom.CoordinatesType argument doesn't match the number of args, which is the same as what geom.NewSequence does.

Option 2

Create new functions for each coordinates type:

  • geom.NewCoordinatesXY(x, y float64) geom.Coordinates
  • geom.NewCoordinatesXYZ(x, y, z float64) geom.Coordinates
  • geom.NewCoordinatesXYM(x, y, m float64) geom.Coordinates
  • geom.NewCoordinatesXYZM(x, y, z, m float64) geom.Coordinates

Usage examples:

  • geom.NewPoint(geom.NewCoordinatesXY(10, 11))
  • geom.NewPoint(geom.NewCoordinatesXYZ(10, 11, 400))

Option 3

The geom.Coordinates are mainly used for the creation of geom.Point values. So maybe we could just create new point constructors like this:

  • geom.NewPointXY(x, y float64) geom.Point
  • geom.NewPointXYZ(x, y, z float64) geom.Point
  • geom.NewPointXYM(x, y, m float64) geom.Point
  • geom.NewPointXYZM(x, y, z, m float64) geom.Point

Usage would look like:

  • geom.NewPointXY(10, 11)
  • geom.NewPointXYZ(10, 11, 400)

There are already "special" constructors for points, e.g. geom.NewEmptyPoint, so there is definitely precedence for this sort of thing.


I'm sort of leaning towards option 3 as the best, but would love to hear your thoughts @akhenakh.

peterstace avatar Mar 14 '24 02:03 peterstace

Thanks for your answer!

I agree with you, option 3 looks nicer to use, but does not really align with the rest of functions where we need to pass a geom.DimXY as argument. So my preference is 3, but consistency tends more to 1.

Maybe both option 1 AND 3 is right? NewPoint just being an helper.

akhenakh avatar Mar 14 '24 15:03 akhenakh