Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Improve LatLngBounds #3803

Merged
merged 1 commit into from
Feb 3, 2016
Merged

Improve LatLngBounds #3803

merged 1 commit into from
Feb 3, 2016

Conversation

jfirebaugh
Copy link
Contributor

Replace:

  • LatLngBounds() (constructor)
  • LatLngBounds::getExtendable() (static)
  • LatLngBounds(sw, ne) (constructor)

With:

  • LatLngBounds::world() (static)
  • LatLngBounds::empty() (static)
  • LatLngBounds::fromCorners(a, b) (static, swaps coordinates as necessary)

@jfirebaugh
Copy link
Contributor Author

@1ec5 With respect to the above, how should MGLLatLngBoundsFromCoordinateBounds behave? Is MGLCoordinateBounds expected to enforce sw < ne, or be lenient about order?

@jfirebaugh
Copy link
Contributor Author

@1ec5 Also, if LatLngBounds does not have a default constructor, how do you specific a default initializer for this:

@implementation MGLMultiPoint
{
    CLLocationCoordinate2D *_coords;
    size_t _count;
    mbgl::LatLngBounds _bounds;
}

@1ec5
Copy link
Contributor

1ec5 commented Feb 3, 2016

MGLCoordinateBounds itself is lenient about order.

As ivars in Objective-C are initialized implicitly, I don’t think it’ll be possible to keep a LatLngBounds as an ivar if the default initializer is removed. In this case, it’d be perfectly fine to have this class hold an MGLCoordinateBounds instead.

@jfirebaugh jfirebaugh self-assigned this Feb 3, 2016
@jfirebaugh jfirebaugh changed the title Refactor LatLngBounds constructors Improve LatLngBounds Feb 3, 2016
static LatLngBounds getExtendable() {
LatLngBounds bounds;
return { bounds.ne, bounds.sw };
static LatLngBounds hull(const LatLng& a, const LatLng& b) {
Copy link
Contributor

Choose a reason for hiding this comment

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

The naming of this method isn’t incredibly obvious. Could you add a documentation comment so its purpose is clear?

@1ec5
Copy link
Contributor

1ec5 commented Feb 3, 2016

The new cardinal direction getters for LatLngBounds are lovely. 👍

* Use "named constructors": empty, world, hull
* Make the two-argument constructor lenient (i.e., it is a hull operation)
* Add various accessors
* Enforce a single empty representation
@jfirebaugh jfirebaugh merged commit 37c27f3 into master Feb 3, 2016
@jfirebaugh jfirebaugh deleted the bounds-api branch February 3, 2016 17:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants