Skip to content

Conversation

@kmadsen
Copy link
Contributor

@kmadsen kmadsen commented May 1, 2020

I'm looking to use some of these functions but want to use List directly.

@kmadsen kmadsen force-pushed the km-turf-measurement-function-refactor branch from 09416f9 to 7230568 Compare May 1, 2020 15:37
@langsmith
Copy link

@kmadsen , can you say more about how this pr came about? Is there a reason why you want another version of TurfMeasurement.along()?

We try to keep the Turf porting as close to turf.js as possible. Per http://turfjs.org/docs/#along docs, this method takes a LineString.

If you've got a list of Point objects, TurfMeasurement.along(LineString.fromLngLats(coords)) should work for you, no?

@kmadsen
Copy link
Contributor Author

kmadsen commented May 1, 2020

@langsmith thanks for the reference http://turfjs.org/docs. There's a few things that are there, not in our mapbox-java that I would like to use!

The reason I was looking to simplify the interface a little is because I'm using it over here: https://github.com/mapbox/mapbox-navigation-android/pull/2802/files#diff-a4ab863681b076878569a40fd6ca5c45R36

So I'm already using along as it is.

 val point = TurfMeasurement.along(LineString.fromLngLats(segmentRoute), distanceMeters, TurfConstants.UNIT_METERS)

buy why convert back and forth from LineString?

 val point = TurfMeasurement.along(segmentRoute, distance, TurfConstants.UNIT_METERS)

So yeah, this pull request is a suggestion. There are already public interfaces to Point in there. It seems the decision was to hide the List? Maybe because turf original interface comes from javascript?

Copy link

@langsmith langsmith left a comment

Choose a reason for hiding this comment

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

Thanks for the explanation @kmadsen 🚀 👍

@kmadsen kmadsen merged commit d9b3c1b into master May 1, 2020
@kmadsen kmadsen deleted the km-turf-measurement-function-refactor branch May 1, 2020 18:35
@kmadsen kmadsen mentioned this pull request May 12, 2020
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants