-
Notifications
You must be signed in to change notification settings - Fork 76
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
Kotlin implementation of https://github.com/heremaps/flexible-polylin… #79
base: master
Are you sure you want to change the base?
Conversation
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. |
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 <marlon.hille@here.com>
…me code clean-up Signed-off-by: Hille, Marlon <marlon.hille@here.com>
Fixed the missing commit sign-off. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Should be polished a bit more.
e950d05
to
38110f5
Compare
Signed-off-by: Hille, Marlon <marlon.hille@here.com>
…able Signed-off-by: Hille, Marlon <marlon.hille@here.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the changes they look good. However, it could be polished a bit more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
… 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 <marlon.hille@here.com>
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).