Skip to content
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

Add methods to check geometry type and convert simultaneously #423

Merged
merged 3 commits into from
Nov 1, 2021

Conversation

peterstace
Copy link
Owner

Description

  • Renames the AsXYZ methods of the Geometry type to MustAsXYZ. This
    follows the go convention that methods and functions prefixed with Must may
    panic if preconditions are not met. Note that there's no change in behaviour
    here, it's simply a rename (these methods previously panicked).

  • Adds new methods named AsXYZ to the Geometry type. These methods have the
    signature AsXYZ() (XYZ, bool). The boolean return value indicates if the
    conversion was successful or not. These methods are useful because they allow
    concrete geometries to be extracted from a Geometry value, with the check
    for the type and the extraction only specified once. Previously, users would
    have to first call IsXYZ to check if the type is expected, and then
    conditionally call AsXYZ. Users now just have to call AsXYZ, and can
    check the flag. This helps to eliminate the class of bugs there the type
    specified with IsXYZ differs from the type specified by AsXYZ (e.g. if
    the user calls IsPoint but then AsPolygon, which would be erroneous).

Check List

Have you:

  • Added unit tests? Yes.

  • Add cmprefimpl tests? (if appropriate?) N/A

  • Updated release notes? (if appropriate?) TODO

Related Issue

Benchmark Results

N/A

This reflects the (unchanged) fact that they panic if the Geometry value
they are called on isn't actually of the specified type. It also opens
up the way to reuse that method name for the `AsFoo() (Foo, bool)`
variant.
@peterstace peterstace self-assigned this Oct 29, 2021
Copy link
Collaborator

@albertteoh albertteoh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@peterstace
Copy link
Owner Author

Thanks for the review! 😄

@peterstace peterstace merged commit 89eca03 into master Nov 1, 2021
@peterstace peterstace deleted the as_geometry_type branch November 1, 2021 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants