feat(library): Port PolyUtil to Kotlin and enhance tests #1565
Conversation
This commit ports the `MathUtil` class from Java to idiomatic Kotlin. The existing tests were leveraged to ensure that the port was successful, and a new test was added for `MathUtil` to ensure that it is well-tested.
This commit ports the `SphericalUtil` class from Java to idiomatic Kotlin. The existing tests were leveraged to ensure that the port was successful, and the tests were updated to use Google Truth assertions.
This commit introduces a significant modernization of the `PolyUtil` class by porting it from Java to idiomatic Kotlin. This change improves the code's conciseness, readability, and null safety. The key changes in this commit are: - **Porting `PolyUtil` to Kotlin**: The `PolyUtil` class has been completely rewritten in Kotlin, taking advantage of features like top-level functions, default arguments, and extension functions. The public API has been preserved with `@JvmStatic` annotations to ensure backward compatibility for Java consumers. - **Updating tests to use Google Truth**: The corresponding `PolyUtilTest` has been updated to use Google Truth assertions. This makes the tests more readable and expressive.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Code Coverage
|
kikoso
left a comment
kikoso
left a comment
There was a problem hiding this comment.
Looks really good! One thought is if we want to incorporate at this point a wrapper for LatLng, but it can be introduced when more classes are migrated.
Co-authored-by: Enrique López-Mañas <[email protected]>
# [3.16.0](v3.15.0...v3.16.0) (2025-08-26) ### Features * **library:** Port PolyUtil to Kotlin and enhance tests ([#1565](#1565)) ([90c56df](90c56df))
|
🎉 This PR is included in version 3.16.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 3.16.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
1 similar comment
|
🎉 This PR is included in version 3.16.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
| * using either geodesic (great circle) or rhumb (loxodromic) paths. | ||
| */ | ||
| object PolyUtil { | ||
| private const val DEFAULT_TOLERANCE = 0.1 // meters |
There was a problem hiding this comment.
@dkhawk DEFAULT_TOLERANCE used to be public API, now it's private. Unintentional breaking change?
There was a problem hiding this comment.
Hi @bubenheimer, apologies for the delay. This should have been marked, in fact, as a breaking change.
There was a problem hiding this comment.
Thanks, @kikoso, I wish there was some documentation why 0.1 as a default on various APIs in this and related libs, rather than a seemingly arbitrary constant.
There was a problem hiding this comment.
@bubenheimer , 0.1 is generally used as a de facto tolerance value when using the Douglas-Peucker algorithm, which is the one used in the Polyline. Agree, this should be clarified in the documentation
There was a problem hiding this comment.
Thank you, that is a good rationale!
There was a problem hiding this comment.
@kikoso At the same time, in my opinion, if it is a commonly used constant with that algorithm, then it is meaningful for this API, and should remain a part of the public API, not just some default value. For example, in my case, I try to use accuracy of device location as the tolerance value for some of these APIs, but when accuracy is unavailable, I need to default to something else, i.e. the 0.1 constant, now with an added comment, rather than call the method with a different signature to automatically get the default, which is even more obscure.
There was a problem hiding this comment.
Adding to that, it appears the DEFAULT_TOLERANCE constant isn't actually used by the Douglas-Peucker implementation unless it's explicitly passed in.
This isn't a new change with the Kotlin port; the previous Java implementation had the same behavior (see PolyUtil.java): https://github.com/googlemaps/android-maps-utils/blob/0bb1d348290ee02c74c02fef13e0e560b4bd370f/library/src/main/java/com/google/maps/android/PolyUtil.java
The current simplify() method still requires the tolerance as an argument, so users would only be using that 0.1 value if they were passing DEFAULT_TOLERANCE in manually. The tolerance value is the primary lever for simplification and is highly use-case-dependent.
Low Tolerance (e.g., 0.1): In my testing, this was often overkill and didn't provide a meaningful reduction in points.
Higher Tolerance (e.g., 5.0 - 10.0): This offered dramatic point savings. While it does create a noticeable (but often acceptable) change in the polyline's shape, it's very effective for things like GPS track rendering (which often has overly dense location data).
There was a problem hiding this comment.
And FWIW, we made the constant public again: #1619
There was a problem hiding this comment.
Thank you, @dkhawk, for these valuable details, which may come in handy for me in the future. At this time I've been using DEFAULT_TOLERANCE just as a fallback for isLocationOnEdge(), but I did not heavily test it. I should use simplify() as well, though, as I need to constrain polygon complexity for performance.
4 participants
This commit introduces a significant modernization of the
PolyUtilclass by porting it from Java to idiomatic Kotlin. This change improves the code's conciseness, readability, and null safety.The key changes in this commit are:
PolyUtilto Kotlin: ThePolyUtilclass has been completely rewritten in Kotlin, taking advantage of features like top-level functions, default arguments, and extension functions. The public API has been preserved with@JvmStaticannotations to ensure backward compatibility for Java consumers.PolyUtilTesthas been updated to use Google Truth assertions. This makes the tests more readable and expressive.