Kotlin implementation of https://github.com/heremaps/flexible-polylin… #79
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
|
Hi, Is there a specific reason why you used the AtomicX classes? An old version of the Java implementation had them and let's say just that I am glad that we got that part completely re-written. |
Author
|
Thank you for the hint. I created the first implemenation some time ago, but never got to contribute it to this project. I now refactored it to also remove the atomic.* references. A nice side effect - the code performance improved. |
…e/tree/master/java Note: There are currently still dependencies to java.util.concurrent.atomic which should be replaced with kotlin.native.concurrent or a project like https://github.com/Kotlin/kotlinx-atomicfu in case the code shall be used with a kotlin multiplatform project. Performance on JVM seems worse (about 4x) than pure Java implementation; so there is room for improvement. Signed-off-by: Hille, Marlon <[email protected]>
…me code clean-up Signed-off-by: Hille, Marlon <[email protected]>
Author
|
Fixed the missing commit sign-off. |
cio
requested changes
Jun 17, 2024
Contributor
cio
left a comment
cio
left a comment
There was a problem hiding this comment.
Looks good. Should be polished a bit more.
moo24
force-pushed
the
master
branch
2 times, most recently
from
e950d05 to
38110f5
Compare
Signed-off-by: Hille, Marlon <[email protected]>
…able Signed-off-by: Hille, Marlon <[email protected]>
cio
reviewed
Jul 1, 2024
Contributor
cio
left a comment
cio
left a comment
There was a problem hiding this comment.
Thank you for the changes they look good. However, it could be polished a bit more.
… Iterator interface Cleanup FlexiblePolyline.Converter and FlexiblePolyline.ThirdDimension as there is no need to use the package name Cleanup of comments Using an Exception with more details in case loading a test case failed Signed-off-by: Hille, Marlon <[email protected]>
cio
approved these changes
Aug 8, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Kotlin implementation of Flexible Polyline encoding based on Java implementation.
API wise, I implemented the Dart API as I prefer that one.
Note: There are currently still dependencies to java.util.concurrent.atomic which should be replaced with kotlin.native.concurrent or a project like https://github.com/Kotlin/kotlinx-atomicfu in case the code shall be used with a kotlin multiplatform project.
Tests are passing. Performance on JVM seems worse (about 4x) than pure Java implementation; so there is room for improvement. However, intention is to provide a kotlin implementation in case someone needs it (e.g. for Kotlin native projects).