Port to Kotlin Multiplatform #25

Ercalvez · 2024-02-25T14:42:53Z

Work still need to be done:

Publishing and signing setup

library-android has been moved to androidMain
FileSystem.SYSTEM is not available in commonMain.

Locale.kt: WIP

# Conflicts: # library/src/main/java/de/westnordost/osmfeatures/CollectionUtils.kt # library/src/main/java/de/westnordost/osmfeatures/FeatureDictionnary.kt # library/src/main/java/de/westnordost/osmfeatures/StartsWithStringTree.kt

# Conflicts: # library/src/main/java/de/westnordost/osmfeatures/ContainedMapTree.kt

# Conflicts: # library/src/commonMain/kotlin/de.westnordost.osmfeatures/BaseFeature.kt # library/src/commonMain/kotlin/de.westnordost.osmfeatures/CollectionUtils.kt # library/src/commonMain/kotlin/de.westnordost.osmfeatures/ContainedMapTree.kt # library/src/commonMain/kotlin/de.westnordost.osmfeatures/Feature.kt # library/src/commonMain/kotlin/de.westnordost.osmfeatures/FeatureDictionnary.kt # library/src/commonMain/kotlin/de.westnordost.osmfeatures/FeatureTagsIndex.kt # library/src/commonMain/kotlin/de.westnordost.osmfeatures/FeatureTermIndex.kt # library/src/commonMain/kotlin/de.westnordost.osmfeatures/FileAccessAdapter.kt # library/src/commonMain/kotlin/de.westnordost.osmfeatures/FileSystemAccess.kt # library/src/commonMain/kotlin/de.westnordost.osmfeatures/GeometryType.kt # library/src/commonMain/kotlin/de.westnordost.osmfeatures/IDBrandPresetsFeatureCollection.kt # library/src/commonMain/kotlin/de.westnordost.osmfeatures/IDLocalizedFeatureCollection.kt # library/src/commonMain/kotlin/de.westnordost.osmfeatures/IDPresetsJsonParser.kt # library/src/commonMain/kotlin/de.westnordost.osmfeatures/IDPresetsTranslationJsonParser.kt # library/src/commonMain/kotlin/de.westnordost.osmfeatures/JsonUtils.kt # library/src/commonMain/kotlin/de.westnordost.osmfeatures/Locale.kt # library/src/commonMain/kotlin/de.westnordost.osmfeatures/LocalizedFeature.kt # library/src/commonMain/kotlin/de.westnordost.osmfeatures/LocalizedFeatureCollection.kt # library/src/commonMain/kotlin/de.westnordost.osmfeatures/PerCountryFeatureCollection.kt # library/src/commonMain/kotlin/de.westnordost.osmfeatures/StartsWithStringTree.kt # library/src/commonMain/kotlin/de.westnordost.osmfeatures/StringUtils.kt # library/src/commonTest/kotlin/de.westnordost.osmfeatures/CollectionUtilsTest.kt # library/src/commonTest/kotlin/de.westnordost.osmfeatures/FeatureDictionaryTest.kt # library/src/commonTest/kotlin/de.westnordost.osmfeatures/FeatureTagsIndexTest.kt # library/src/commonTest/kotlin/de.westnordost.osmfeatures/FeatureTermIndexTest.kt # library/src/commonTest/kotlin/de.westnordost.osmfeatures/IDBrandPresetsFeatureCollectionTest.kt # library/src/commonTest/kotlin/de.westnordost.osmfeatures/IDLocalizedFeatureCollectionTest.kt # library/src/commonTest/kotlin/de.westnordost.osmfeatures/IDPresetsJsonParserTest.kt # library/src/commonTest/kotlin/de.westnordost.osmfeatures/IDPresetsTranslationJsonParserTest.kt # library/src/commonTest/kotlin/de.westnordost.osmfeatures/JsonUtilsTest.kt # library/src/commonTest/kotlin/de.westnordost.osmfeatures/LivePresetDataAccessAdapter.kt # library/src/commonTest/kotlin/de.westnordost.osmfeatures/StartsWithStringTreeTest.kt # library/src/commonTest/kotlin/de.westnordost.osmfeatures/TestLocalizedFeatureCollection.kt # library/src/commonTest/kotlin/de.westnordost.osmfeatures/TestPerCountryFeatureCollection.kt # library/src/commonTest/kotlin/de.westnordost.osmfeatures/TestUtils.kt

westnordost

A very incomplete review while in the train. I think when home I'll rather check out the branch and correct all the small issues I find myself and only note the bigger ones (if any) here on github, more efficient that way.

It's a good idea to have the Android-specific stuff in the Android-part of the library itself, makes sense.

Four general comments:

The directory name looks suspect: library/src/commonMain/kotlin/de.westnordost.osmfeatures/... - i.e. why are there dots in the name instead of subdirectories? It's the first time that I see this. I checked the Kotlin coding conventions and didn't see that mentioned.
Regarding the build.gradle.kts: I recently created another kotlin multiplatform library, maybe you could copy it from there: https://github.com/westnordost/osm-opening-hours
~~Some fields, parameters and variables you made nullable even though this looks like an oversight. In case it is not, comments should be added to the source to explain / document it.~~
With kotlinx-serialization-json it is not necessary (nor recommended, as far as I know) to do JSON parsing by hand. You can simply annotate the data model how it should be (de)serialized. See https://kotlinlang.org/docs/serialization.html#serialize-and-deserialize-json . Did you not know this, or is there some reason why you chose to parse it by hand?

library/src/commonMain/kotlin/de.westnordost.osmfeatures/Feature.kt

library/src/commonMain/kotlin/de.westnordost.osmfeatures/BaseFeature.kt

library/src/commonMain/kotlin/de.westnordost.osmfeatures/Feature.kt

library/src/commonMain/kotlin/de.westnordost.osmfeatures/LocalizedFeature.kt

library/src/commonMain/kotlin/de.westnordost.osmfeatures/StartsWithStringTree.kt

library/src/commonMain/kotlin/de.westnordost.osmfeatures/StringUtils.kt

library/src/commonMain/kotlin/de.westnordost.osmfeatures/ContainedMapTree.kt

library/src/commonMain/kotlin/de.westnordost.osmfeatures/BaseFeature.kt

…ionary

westnordost · 2024-05-15T11:38:41Z

Alright, merged. In the end, the line of code count is only 72% of what we had for Java. 🎉

Since we made breaking changes to the API anyway (Locale -> String), I also changed a few more things that were bothering me.

For the time being, the initialization of the multiplatform version is about 15% slower than before. I think this is mainly due to whole file read into a string first, then into a DOM and then only in the final data structure. At least the first step should be solvable once kotlinx-serialization-json supports directly reading kotlinx.io.Source.

Ercalvez · 2024-05-15T16:00:53Z

Alright, merged. In the end, the line of code count is only 72% of what we had for Java. 🎉

Since we made breaking changes to the API anyway (Locale -> String), I also changed a few more things that were bothering me.

For the time being, the initialization of the multiplatform version is about 15% slower than before. I think this is mainly due to whole file read into a string first, then into a DOM and then only in the final data structure. At least the first step should be solvable once kotlinx-serialization-json supports directly reading kotlinx.io.Source.

Wow, 72% reduction is impressive 🎉

westnordost · 2024-05-15T16:43:47Z

Nono, reduction TO 72%

Ercalvez added 26 commits November 20, 2023 02:46

Ported Feature and BaseFeature to Kotlin

916052c

Ported Feature and BaseFeature to Kotlin

b98a883

Locale.kt: WIP

Rename .java to .kt

f64ae87

kmp WIP, use okio for I/O

b3cf2b2

Rename .java to .kt

47c30a4

trying to fix unit test

3b53d73

Fixed file access tests + Locale equality

37258f4

Rename .java to .kt

788dfdb

Fixed dissectCountryCode

28b420d

Fixed dissectCountryCode

b4aca73

Merge remote-tracking branch 'origin/kmp' into kmp

6b067f9

# Conflicts: # library/src/main/java/de/westnordost/osmfeatures/CollectionUtils.kt # library/src/main/java/de/westnordost/osmfeatures/FeatureDictionnary.kt # library/src/main/java/de/westnordost/osmfeatures/StartsWithStringTree.kt

Fixed remove useless trycatch

145f846

Rename .java to .kt

102c10a

Converted all files to Kotlin

d3a45d5

Converted all files to Kotlin

3832f45

Merge remote-tracking branch 'origin/kmp' into kmp

efa97a9

# Conflicts: # library/src/main/java/de/westnordost/osmfeatures/ContainedMapTree.kt

Fix build

49f0c9b

fix resource path

08b67d8

cleaned imports and unused files

0355d0f

cleaned imports and unused files

86a375d

fixed getAll methods in tests

55c4d61

fix excludeCountryCodes unit tests + dependencies

2382034

fix excludeCountryCodes unit tests + dependencies

e2d344b

fix merge conflict

f71a920

fix build gradle

7d7b1fd

westnordost reviewed Feb 25, 2024

View reviewed changes

westnordost added 3 commits February 25, 2024 19:07

Apply suggestions from code review

5a35637

Shorter code

2dad8c1

add comment

3391252

westnordost added 25 commits May 10, 2024 20:20

correct synchronization / lazynization

5b9c5fc

Classes from kotlinx-io-core are on the interface

0fec5c7

add entry for changelog

46bc930

fix tests

b7ecd80

should be internal

8316895

fix android build

6d1c037

update readme

4b0b7a8

prettify parsers

2004dc1

more parser stuff

0c99a57

remove todo

aeaee48

add test also for brand data

71b7143

remove unnecessary map

ab9037e

don't create a new regex every time that function is called

29bbe6c

naming

dad3413

rename file

89f893a

add test and fix

7354d42

declare io exception can be thrown

4fdd7a2

make getters public

5b82c63

update readme

2f5c4e4

fix building names and terms index

cc75a24

fix tests

3e4715a

add link to ticket to monitor

b6f3e0c

is is more correct to use "language" rather than "locale"

e58ce94

builders should only be constructable from the methods of FeatureDict…

6572300

…ionary

Merge branch 'master' into kmp

7e66378

westnordost merged commit b51f189 into westnordost:master May 15, 2024

westnordost mentioned this pull request Jul 29, 2024

MapLibre Native Compose Multiplatform Library maplibre/maplibre-native#2638

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Port to Kotlin Multiplatform #25

Port to Kotlin Multiplatform #25

Ercalvez commented Feb 25, 2024

westnordost left a comment •

edited

Loading

westnordost commented May 15, 2024

Ercalvez commented May 15, 2024

westnordost commented May 15, 2024

Port to Kotlin Multiplatform #25

Port to Kotlin Multiplatform #25

Conversation

Ercalvez commented Feb 25, 2024

westnordost left a comment • edited Loading

Choose a reason for hiding this comment

westnordost commented May 15, 2024

Ercalvez commented May 15, 2024

westnordost commented May 15, 2024

westnordost left a comment •

edited

Loading