Refactoring Zone classes and ideas for functionality #129
Moritz-Schmidt
started this conversation in
Ideas
Replies: 1 comment 4 replies
-
I sure like the idea of an abstract base class, it is the proper way to extend the functionality. I personally wasn't aware, that abstraction can be achieved with python 😀. But I do not like adding the http as a dependency to the zone class on the other hand. The current decoupled implementation has a good separation of concerns. Moving the initialization to the init method would make sense IMO, because I do not really like the kwargs feature too. It is again a lack of my python knowledge, whereas I thought this would be the python way to do initialization 😄 |
Beta Was this translation helpful? Give feedback.
4 replies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
-
Currently, the
TadoXZone
Class inherits from theTadoZone
Class which gives theTadoXZone
class a lot of attributes which are not used by Tado X. Also thefrom_data
function is quite complex with thekwargs
dict which also doesn't ensure typing.I want to propose creating an abstract base class which contains all the common attributes and having the
TadoZone
and theTadoXZone
both inherit from the common abstract base class. To simplify thefrom_data
method I'd suggest omitting it and doing the logic in the__init__
function. This can be done backwards-compatible by initializing the Class from thefrom_data
function with directly passing the data object to the__init__
function.That way default values can be set in the attribute definitions of the class. I think this would reduce the overall lines of code and increase readability. It would also make it possible to correctly type it.
I've also had the Idea to use the Zone classes not only as a data structure but also for all the zone-related functionality. This however means that the zone class would need to have access to the Http instance. This Idea is also a bit contradictory to the Idea above since the
__init__
function would need to have another functionality.A example what the later Idea would look like:
This approach could be backward-compatible; for instance,
Tado.set_overlay
might internally use the zone class:I look forward to feedback on both the abstract base class proposal and the potential expansion of zone class functionality.
Beta Was this translation helpful? Give feedback.
All reactions