Parse String to UUID #1006

[EmmanuelBerkowicz]

Summary

Implements UUID parsing support for DataColumn.parse() as per open issue #1006

Changes

• Added UUID parser to parsersOrder list in main/../parse.kt using java.util.UUID.fromString() with exception handling
• Added test cases in test/../parse.kt:
  • Valid UUID strings are parsed to UUID objects
  • Invalid UUID strings remain String type

Testing

• Both positive and negative test cases pass
• Unable to run full test suite due to Java version compatibility issue with simple-git plugin (unrelated to this change)

[EmmanuelBerkowicz]

Hey @Jolanrensen,
I'm still pretty new to open source, is there an issue?

[Jolanrensen]

Thank you for your addition! :)

Aside from my comment, I'd also reccomend running the linter task "ktlintFormat" across the project. This will fix some newlines and indentations, so they're consistent with the rest of the project.
Our CI runs "build" with all tests and "ktlintCheck", which now fails.

[Jolanrensen]

While this approach works, the parse() function can make this piece of code be called very often (if you try to parse a column with 100 Strings, it gets called 100 times). Exceptions are meant for exceptional cases, not for default flows, as they are quite heavy to throw (a stacktrace needs to be built every time).
For this reason, we tried to avoid throwing exceptions for the other types we can parse, like dates etc. I think for UUIDs we can do the same thing :)

We could namely check the string by a Regex first, return null when the regex doesn't match, and call UUID.fromString() only when the regex does match. You can keep the try {} catch() around it for safety of course. This will save a lot of stack traces being created :)

I found this regex: "[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}" in several places online; I think it's safe to use it, but if you're unsure, please add some more tests.

[Jolanrensen]

Hey @Jolanrensen, I'm still pretty new to open source, is there an issue?

No issue :) I self-requested a review so I can comment on some particular parts of the PR. If you correct it and our test suite and the team is okay with it, we can merge it to the master

[Jolanrensen]

Actually, as my collegue just pointed out to me, the idea was to use Kotlin's new experimental builtin Uuid class https://kotlinlang.org/api/core/kotlin-stdlib/kotlin.uuid/ instead of the Java variant.

It works very similarly but is more Kotlinlike :). Feel free to OptIn to @ExperimentalUuidApi, I'll add a ParserOption for our users to let them decide whether they want to use the experimental UUID parser or not after you finish this part.

You can use the same Regex approach I mentioned, but as it's Kotlin itself, I'll also ask whether they can create a parseOrNull() function without exceptions :)

[EmmanuelBerkowicz]

Thanks for the feedback @Jolanrensen !

I'll work on this when I get home and update!

Emmanuel