Skip to content

Conversation

dkhawk
Copy link
Contributor

@dkhawk dkhawk commented Aug 29, 2025

This pull request refactors several core data classes from Java to Kotlin. This modernization improves code readability, safety, and conciseness by leveraging Kotlin features like properties, val, and non-nullable types.

Summary of Changes

  • DataPolygon.java -> DataPolygon.kt: Converted the DataPolygon interface to Kotlin. It now uses val properties for outerBoundaryCoordinates and innerBoundaryCoordinates.
  • Feature.java -> Feature.kt: Converted the Feature class to Kotlin. This includes:
    • Replacing Java getters and setters (getGeometry, getProperties, hasProperty) with Kotlin properties (geometry, properties, hasProperty).
    • Using a private backing property (_properties) for the mutable map to expose an immutable view.
  • Geometry.java -> Geometry.kt: Converted the Geometry interface to Kotlin, using val properties for geometryType and geometryObject.
  • Layer.java -> Layer.kt: Converted the Layer abstract class to Kotlin. It now uses generic types for Feature and uses Kotlin's property access syntax for renderer (renderer.map instead of renderer.getMap()).
  • LineString.java -> LineString.kt: Converted the LineString class to Kotlin, using val properties for its data and adding equals and hashCode implementations.
  • MultiGeometry.java -> MultiGeometry.kt: Converted the MultiGeometry class to Kotlin. The constructor now enforces non-nullability, and the getGeometryType method is replaced with an override property.
  • Point.java -> Point.kt: Converted the Point class to Kotlin, using val properties for its data and including equals, hashCode, and toString overrides.

@googlemaps-bot
Copy link
Contributor

googlemaps-bot commented Aug 29, 2025

Code Coverage

Overall Project 40.52% -0.69% 🍏
Files changed 84.04% 🍏

File Coverage
GeoJsonMultiPoint.java 100% 🍏
GeoJsonMultiPolygon.java 100% 🍏
GeoJsonGeometryCollection.java 100% 🍏
GeoJsonMultiLineString.java 100% 🍏
SphericalUtil.kt 99.72% 🍏
PolyUtil.kt 97.88% -0.07% 🍏
Point.kt 93.75% -6.25% 🍏
Feature.kt 91.38% -8.62% 🍏
LineString.kt 89.06% -10.94% 🍏
MultiGeometry.kt 72% -28% 🍏
Layer.kt 69.23% -30% 🍏
GeoJsonFeature.java 60.09% -3.29%
KmlPolygon.java 53.52% 🍏
GeoJsonLayer.java 42.11% 🍏
KmlMultiGeometry.java 38.89% 🍏
GeoJsonRenderer.java 35.68% -13.51%
Renderer.java 16.74% 🍏
KmlRenderer.java 3.13% -3.23%
KmlLayer.java 0% -1.24%

@dkhawk dkhawk changed the title Feat/port data package to kotlin chore: Convert data types to Kotlin Aug 29, 2025
@dkhawk dkhawk requested a review from kikoso August 29, 2025 22:39
This commit modernizes the `Point` class by converting it from Java to idiomatic Kotlin, improving null safety, readability, and code conciseness. The corresponding tests have also been migrated and improved.

Key changes:
- **Port `Point` to Kotlin**: The `Point` data class has been converted to Kotlin, leveraging its features for a more robust implementation.
- **Migrate and Enhance `PointTest`**: The `PointTest` class is now in Kotlin and uses the Google Truth assertion library for more expressive tests.
- **Improved Test Coverage**: New tests for `equals()`, `hashCode()`, and `toString()` have been added. The nullability test was updated to use reflection to verify Java interoperability, confirming that a `NullPointerException` is thrown for null constructor arguments.
- **Update `GeoJsonPointTest`**: A related test was updated to expect a `NullPointerException` instead of an `IllegalArgumentException`, aligning with the behavior of the new Kotlin-based `Point` superclass.
- **Update Copyright Headers**: Copyright years were updated in the modified files.
This commit modernizes the `LineString` class and its corresponding tests by converting them from Java to idiomatic Kotlin. This change improves code conciseness, readability, and null safety.

The key changes are:
- **Port `LineString` to Kotlin**: The `LineString` class has been completely rewritten in Kotlin.
- **Port `LineStringTest` to Kotlin**: The test class for `LineString` has been converted to Kotlin and updated to use Google Truth for assertions, making the tests more expressive.
- **Enhance Test Coverage**: Added tests for `equals()`, `hashCode()`, and `toString()` to ensure correctness.
- **Update Related Tests**: The `GeoJsonLineStringTest` was updated to catch `NullPointerException` instead of `IllegalArgumentException`, reflecting the stricter null-safety of the new Kotlin-based `LineString` constructor.
This commit refactors the `DataPolygon` interface from Java to Kotlin.

Key changes:
- The `DataPolygon` interface is now a Kotlin file, with `get...()` methods converted to idiomatic Kotlin properties.
- `KmlPolygon` has been updated to use the `List` interface instead of the concrete `ArrayList` in its implementation of `DataPolygon`.
- The copyright year has been updated in `DataPolygon.kt`.
This commit modernizes the `Feature` class by porting it from Java to idiomatic Kotlin, improving its conciseness, readability, and null safety.

The key changes include:
- **Porting `Feature` to Kotlin**: The class is now written in Kotlin, using modern language features like properties and a primary constructor.
- **Correct Observable Behavior**: Setters for geometry and properties now correctly call `setChanged()` and `notifyObservers()`, ensuring that observers are notified of modifications.
- **Test Modernization**: The corresponding `FeatureTest` has been ported to Kotlin and now uses Google Truth for more expressive assertions.
- **Enhanced Test Coverage**: New tests have been added to verify the `Observable` behavior when a feature's properties or geometry are changed.
- **Subclass Update**: `GeoJsonFeature` has been updated to align with the changes in its `Feature` superclass.
This commit refactors the `Geometry` interface, converting it from Java to Kotlin.

As part of this change, the `getGeometryType()` and `getGeometryObject()` methods have been replaced with the idiomatic Kotlin properties `geometryType` and `geometryObject`.

The implementing classes, `Point` and `LineString`, and their corresponding tests have been updated to align with this new property-based API.
This commit refactors the `Layer` class by converting it from Java to idiomatic Kotlin. It also introduces generics to both the `Layer` and `Renderer` classes to improve type safety throughout the data layer.

Key changes:
- **Convert `Layer` to Kotlin**: The `Layer` class is now `Layer.kt`, utilizing Kotlin features like properties and `when` expressions for more concise and readable code.
- **Introduce Generics**: `Layer` and `Renderer` are now generic (`Layer<T : Feature>`, `Renderer<T extends Feature>`). This enforces type constraints at compile time.
- **Improve Type Safety**: Subclasses like `GeoJsonLayer`, `KmlLayer`, `GeoJsonRenderer`, and `KmlRenderer` now extend the generic base classes, eliminating the need for unsafe casts when handling features.
- **Add Unit Tests**: A new `LayerTest.kt` file has been added with comprehensive unit tests for the `Layer` class, using MockK and Truth to verify its behavior.
This commit refactors the `MultiGeometry` class from Java to Kotlin, improving its design, null safety, and immutability.

The key changes are:
- **Porting `MultiGeometry` to Kotlin**: The class is now a generic, immutable Kotlin class. The constructor enforces non-nullability for the list of geometries, changing the exception for null constructor arguments from `IllegalArgumentException` to `NullPointerException`.
- **Updating Subclasses**: All subclasses of `MultiGeometry` (e.g., `GeoJsonMultiPoint`, `KmlMultiGeometry`) have been updated to align with the new Kotlin base class. They now override the `geometryType` property instead of calling a setter.
- **Modernizing Tests**: The `MultiGeometryTest` has been converted to Kotlin and uses Google Truth. Tests for all affected subclasses have been updated to assert the correct exception types.
This commit introduces `Polyline` and `Polygon` as type aliases for `List<LatLng>` to improve code readability and provide stronger semantic meaning. This change makes it explicit whether a list of coordinates represents a line or a closed area.

The following files and classes have been updated to use these new type aliases:
- `PolyUtil`: Functions now accept `Polyline` or `Polygon` instead of a generic `List<LatLng>`.
- `SphericalUtil`: `computeLength` and `computeArea` now operate on `Polyline` and `Polygon` respectively.
- `LineString`: The `coordinates` property is now of type `Polyline`.
- `DataPolygon`: Boundary properties now use the `Polygon` type alias.
This commit refactors the `SphericalUtil` object to improve code clarity and leverage modern Kotlin features.

Key changes include:
- Introduced private `toRadians()` and `toDegrees()` extension functions for `Double` to replace calls to `Math.toRadians()` and `Math.toDegrees()`.
- Converted several methods to more concise single-expression functions.
- Improved the readability of the `computeHeading` function by using more descriptive variable names and clearer logic.
This commit refactors the `SphericalUtil` object to use more modern and idiomatic Kotlin patterns, improving code readability and conciseness.

Key changes include:
- **Functional Approach**: Replaced imperative `for` loops in `computeLength` and `computeSignedArea` with functional constructs like `zipWithNext()` and `sumOf()`.
- **Extension Functions**: Introduced helper extension functions for `LatLng` to enable destructuring (`component1`/`component2`) and simplify conversion to radians (`toRadians`).
- **Code Cleanup**: Refactored methods like `computeOffset`, `computeOffsetOrigin`, and `interpolate` to use immutable variables (`val`) and the new helper functions.
- **New Tests**: Added `SphericalUtilKotlinTest.kt` to provide test coverage for the refactored list-based calculations using Google Truth.
@dkhawk dkhawk force-pushed the feat/port-data-package-to-kotlin branch from 55bea6a to 3148633 Compare September 5, 2025 22:52
This commit refactors the `computeSignedArea` function for better performance and readability.

The implementation now uses a `Sequence` to iterate over the path edges, avoiding the allocation of an intermediate list for the closed path. A local function, `polarArea`, has also been introduced to improve code clarity.
This commit includes minor code cleanup across several files:
- Removes extra newlines in `GeoJson*`, and `Layer` classes.
- Fixes indentation in `PolyUtil`.
- Improves readability in `SphericalUtil.computeSignedArea`
@@ -0,0 +1,61 @@
/*
Copy link
Collaborator

Choose a reason for hiding this comment

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

Some classes are being marked as moved, some others as new classes. I wonder if we could unify this (ideally, with files being marked as moved)

override fun toString(): String {
val geometries = "geometries=$geometryObject"
return "MultiGeometry{$geometries}"
}
Copy link
Collaborator

@kikoso kikoso Sep 15, 2025

Choose a reason for hiding this comment

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

Would we like to have a hashCode() function here?

*/
public String getType() {
return getGeometryType();
@Override
Copy link
Collaborator

Choose a reason for hiding this comment

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

This would introduce a breaking change. It is probably fine when we are done with the migration, but for now we can add it as a new function and deprecate the previous one. What do you think?

*/
public String getType() {
return getGeometryType();
@Override
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same issue with the breaking change.

/**
* A Polygon is a list of LatLngs where each LatLng is a vertex of the polygon.
*/
typealias Polygon = List<LatLng>
Copy link
Collaborator

Choose a reason for hiding this comment

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

I was about to write that we had a breaking change, and whether we could consider an alias. Well done!

@dkhawk dkhawk marked this pull request as draft September 16, 2025 19:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants