Implement nearestPointOn(Multi)Line and internal intersects helper

[skreborn]

The original implementation in Turf.js handles both LineString and MultiLineString objects with a union type. However, since Dart has no support for such, I've opted for creating two separate methods instead of resorting to runtime type checking. This means that there is both a nearestPointOnLine and a nearestPointOnMultiLine. If you'd rather go a different route with this, I'm happy to make the changes.

I haven't implemented Raw versions of these functions as I didn't particularly see the point - I hope that's okay. There are, however, some internal functions and data structures private to nearest_point_on_line.dart, as it makes the implementations cleaner and parts of it reusable within the same file - specifically, nearestPointOnMultiLine uses the implementation of nearestPointOnLine internally.

I've also written tests based on the input and expected output values in the original Turf.js implementation. They all pass.

There is an extra file included within this PR that doesn't strictly belong; intersection.dart contains code that finds the geometrical intersection between two lines. This function has not been exported from the library, but it can be used in the future to implement lineIntersect (and consequently numerous other methods that depend on it). If you'd like, I can submit it as a separate PR.

[lukas-h]

Thank you for you PR – really great work so far! 😃

The original implementation in Turf.js handles both LineString and MultiLineString objects with a union type. However, since Dart has no support for such, I've opted for creating two separate methods instead of resorting to runtime type checking

I could see a use case where you would get data dynamically and don't know which of the two types it is. In this case I think, it would make sense to create a version with GeometryType and runtime type checking. But at the same time, I think we shouldn't get rid of the versions with explicit type!

I haven't implemented Raw versions of these functions as I didn't particularly see the point - I hope that's okay.

This is a major design decision of this lib you are pointing out. It is also – again 😢 –  because Dart does not have union types.
In TS you would just write something like : Position | Point | Feature<Point> but what could be a good solution for Dart?
I haven't found an appropriate solution so far.
Let's stick with your solution for the time being and start a discussion on that topic, maybe with the input of @baparham & @tobrun

There is an extra file included within this PR that doesn't strictly belong; intersection.dart contains code that finds the geometrical intersection between two lines. This function has not been exported from the library, but it can be used in the future to implement lineIntersect (and consequently numerous other methods that depend on it). If you'd like, I can submit it as a separate PR.

I think it's fine if it is in the same PR, but we could maybe point it out in the title.
And what about some dedicated unit tests for the intersects function?

[skreborn]

I could see a use case where you would get data dynamically and don't know which of the two types it is. [...] This is a major design decision of this lib [...] because Dart does not have union types.

I'll open a discussion topic on this shortly - let's see if we can figure something out. Edit: Opened as #58.

I think it's fine if it is in the same PR, but we could maybe point it out in the title. And what about some dedicated unit tests for the intersects function?

I've updated the title and I'll see about those unit tests today.

[skreborn]

I have found that the MultiLineString version of this function was pretty useless considering the fact that there was no way to tell which line within a MultiLineString the nearest point was found on, so I've opted to add an extra line property to the resulting Feature that contains its index.

In the original Turf.js implementation, this is solved differently; it just counts all lines in the index property. I feel like this is cleaner and easier to use for library consumers, but I'm happy to change it if compatibility with the existing Turf.js implementation is more important.

I couldn't implement unit tests for intersects, unfortunately, as it isn't exported from the library and so the test files can't reach it either. Should I export it, go another way with testing, or just leave it be?

[lukas-h]

I have found that the MultiLineString version of this function was pretty useless considering the fact that there was no way to tell which line within a MultiLineString the nearest point was found on, so I've opted to add an extra line property to the resulting Feature that contains its index.

Am I understanding it right, that we basically have a superset oft the functionality of the original implementation, “index” is the same?

This line property is an excellent new idea btw 👍👍👍👍👍🔥

I couldn't implement unit tests for intersects, unfortunately, as it isn't exported from the library and so the test files can't reach it either. Should I export it, go another way with testing, or just leave it be?

But the test folder is in the same package. I think it should be possible to include the implementation file like this:
import 'package:turf/src/intersect.dart'
The linter could warn you not to import implementation (not exported) files, but this is fine in this case imo! Or have I misunderstood the error?

[skreborn]

Am I understanding it right, that we basically have a superset oft the functionality of the original implementation, “index” is the same?

Not exactly. index is the same for a LineString, but for a MultiLineString, Turf.js uses index across all of the lines. Instead, I've opted to make index a local index within the line identified by line.

But the test folder is in the same package. I think it should be possible to include the implementation file [...].

You're right, that's my mistake - you indeed can import it from the same package. I'll get on those tests.

[lukas-h]

Not exactly. index is the same for a LineString, but for a MultiLineString, Turf.js uses index across all of the lines. Instead, I've opted to make index a local index within the line identified by line.

What about keeping index compatible with turf JS, but we could extend the functionality with something like localIndex. wdyt?

[skreborn]

What about keeping index compatible with turf JS, but we could extend the functionality with something like localIndex.

Seems reasonable, I'll look into it!

[lukas-h]

@skreborn

Any update on the index and localIndex idea, wdyt?

Is this PR ready for review anyways?

[skreborn]

I think it's an excellent idea, I just haven't had the time to implement it yet - apologies for that. It needs this modification, unit test for the helper function, and a rebase. I'll get to it as soon as I can.

[lukas-h]

If you‘re okay with it, I will continue your efforts @skreborn in branch feature-nearest-point-on-line

[skreborn]

If you‘re okay with it, I will continue your efforts @skreborn in branch feature-nearest-point-on-line

Of course, and thank you! I really am finding it hard at the moment to work on it myself.

[lukas-h]

closed in favor of #87