Issue with URL encoding

[StefanWinterberger]

Hi First of, thank you for the time you invested in this library. Looks very handy to use.

Issue with Version: V0.4.0.alpha.0

I tried to execute the example code for the current weather, that you have in the wiki and i received an error. When I dig deeper into the code i saw that the query URL was already url-encoded. If I removed the urlEncode(...) in line 101 of Query.kt the example worked for me again. However, i did not try this change with every other possible request.

Did I missunderstand something with the URL encoding, or has there something changed in underlying libraries? Can you reporoduce the issue?

(There was a second issue while parsing the response. There is a new attribute in the is_day of type Int, so I added this as well in ResponseCurrentWeather.kt. But this is not the topic here)

[DadiBit]

Hello, I've been quite busy with my finals in the past months and I couldn't find time to work on the project: I do have a bit of time on this weekend, and I'm going to fix some stuff. I'm working on doing a full refactor to make the library more lightweight (right now it's increasing a simple android app size by megabytes... Not ideal!).

If I removed the urlEncode(...) in line 101 of Query.kt the example worked for me again. (There was a second issue while parsing the response. There is a new attribute in the is_day of type Int, so I added this as well in ResponseCurrentWeather.kt. But this is not the topic here)

Anyway, I fixed both issues. Thanks for the report. The main problem is that I did not document plenty of stuff, thus making the library easy to break + the example didn't reflect a few changes, I believe.

As I'm getting better with serializers, I made one that converts the is_day field from Int to Boolean, making it much easier to use it in boolean operations... Not sure why is_day doesn't directly return true/false, since they do exist in JSON: perhaps it's considered as wasting bandwidth, or maybe it was originally intended to return a wider range of values (enums, or something like that)

[StefanWinterberger]

Thanks for the fast response and the fixes, even thoug you are busy. I'm looking forward for future changes.

[DadiBit]

No problem! You can already have a sneak peek of the refactoring I've been working on this weekend. Hopefully, I'm going to release version 0.6.0 (without alpha or other pre-releases tags) this summer.

Firstly, the total Kotlin SLOC is going to be around ~2k (in version 0.4.0 I believe it was like ~5.5k) lines. Secondly, getting rid of enums for the daily/hourly options should boost performance (not tested, though) and ease queries writing when there are plenty of options. All thanks to the Options.list and the of shorthands:

Options.list(Forecast.Daily) { of(
    weathercode, sunrise, sunset, temperature2mMax, temperature2mMin
) }

Instead of:

listOf(
    Forecast.Daily.weathercode,
    Forecast.Daily.sunrise,
    Forecast.Daily.sunset,
    Forecast.Daily.temperature2mMax,
    Forecast.Daily.temperature2mMin
)

This actually allows to add extra options in the varargs like Options.list(Forecast.Daily, "not_implemented_yet") { of( ... )}, thus letting the dev being much more independent of the encoded values (eg: if the library isn't updated, the user can still access new, unimplemented, options).

Right now, I've removed all endpoints and just refactored the Forecast one. This is mostly due to the fact that data should be obtainable anyway thanks to the models parameter (still needs to be implemented, but should be super-easy) + I believe it's useless to cover all forecast API endpoints: the library is flexible enough to let anyone just create a custom query/response classes and call Endpoint(URL("https://api.open-meteo.com/some-endpoint")).query<ResponseClass, QueryClass>(query).

With all of that said, I think I won't do other refactorings, since I really like the way it's shaping with this cleaner approach, so if you want you can already start looking into the changes (mostly merged fragmented stuff in the common package).