Add new splitQuery API accepting Ktor Url or string#2931
Conversation
fire-light42
left a comment
fire-light42
left a comment
There was a problem hiding this comment.
It works well from testing and the code looks good 👍
I just have a thought regarding the new API surface. This is not blocking. I just want to hear what you think before merging this.
| * // returns {"foo": "bar", "baz": "qux"} | ||
| */ | ||
| @Prerelease | ||
| fun splitQuery(url: Url): Map<String, String> { |
There was a problem hiding this comment.
I would prefer to keep the API-surface somewhat dependency-agnostic. There is no need to make the same mistake as splitQuery(url: java.net.URL) again, when we can just expose a string argument instead.
There was a problem hiding this comment.
What I was aiming for was to expose a way to accept a direct URL as well as String. By passing a URL it can sometimes be a bit more type safe as well, and also as we shift towards ktor URL more, sometimes a URL may be wanted directly, in which case, I just thought it was better and more straightforward to accept a URL with a separate convenience method accepting a string only as well. I am open to changes though if you think it should be done differently?
There was a problem hiding this comment.
I am kinda considering whether this should instead be an extension function on String in StringUtils though, in which case it would make a little more sense to simply drop the URL overload and just accept that extension method. I am honestly not 100% sure on the best option here...
2 participants
No description provided.