Ingenjorsarbete-For-Klimatet / ifk-smhi

SMHI climate data client.
https://ingenjorsarbeteforklimatet.se/ifk-smhi/
MIT License
8 stars 1 forks source link

Reduce method side effects #66

Closed mgcth closed 5 months ago

mgcth commented 1 year ago

Description

Reduce or eliminate (as much as possible) side effects from our methods. Along these lines, remove the Metobs class altogether and just use the individual components.

docNord commented 1 year ago

How do you think the structure should be in order to avoid such side effects, and what side effects are you thinking of?

mgcth commented 1 year ago

We could probably delete this class altogether

https://github.com/Ingenjorsarbete-For-Klimatet/ifk-smhi/blob/6be598b6eba0199b221f237b337887c4951e69fd/src/smhi/metobs.py#L12

For example, get_periods doesn't actually return anything, but just mutates an internal state periods of the object

https://github.com/Ingenjorsarbete-For-Klimatet/ifk-smhi/blob/6be598b6eba0199b221f237b337887c4951e69fd/src/smhi/metobs.py#L74-L93

There are probably more examples.

I think it makes the code harder to test and reason about, and as a user, I would expect get_... to return something and not only modify the object state.

I'd go for pure functions as much as possible, as long as we don't copy data unecessairly but that shouldn't be a big problem as the data here is relatively small for each request. It should simplify any future multithreading/async additions we'd want to add.

docNord commented 1 year ago

So... Should we pencil out a plan for how we want to structure things before we start remodeling? Or do you already have a blueprint like this in mind?

mgcth commented 1 year ago

Maybe we should try to release version 0.1.0 first :)

docNord commented 1 year ago

Good point :)