AppDaemon internals coding style

Hi there, I’m diving into the AppDaemon code to better understand it, and maybe help out implementing my own feature request.

I see many parts of the code that aren’t really “pythonic”, and after many years of python programming it kinda hurts my eyes seeing this practices :sweat_smile:

As an example:

for name in self.app_config:
    if "disable" in self.app_config[name] and self.app_config[name]["disable"] is True:
  1. looping dictionary keys to then get the value by that key is best done with dictionary.items() that already returns the key and value pair
  2. the dictionary key existence can be bypassed by using dictionary.get(key, default) (or even without the default value since it returns None by …default)
  3. the if condition is True can be simplified in if condition to leverage the boolness of any object; it will also evaluate to true for empty lists, dictionary and strings.

With that in mind, the code can be rewritten to

for name, cfg in self.app_config.items():
    if cfg.get("disable"):

Also, many methods are really long and it’s difficult to track what’s going on.

This post is not to criticize what’s been done so far, but it is simply to ask:

  • where this coding style decisions made because of better performances? In my projects I prefer readability over optimization, so I don’t know if there’s some benchmarking done behind this
  • If the answer of the previous question is no, are you open to accept PRs that alter significantly the code to make it “cleaner”?

I personally leverage the sourcery plugin (for vscode, pycharm and other IDEs) to detect “bad” code and refactor it, it is really handy for this kind of work.

Other than the style, I also have an architecture related question: why use the AppDaemon class to hold all the other classes and pass it to them, instead of explicitly define and store the dependencies of a component inside its __init__ method?
If i understand correctly, the components are created only once, so the self.AD.component will always return the same instance.
A classmethod from_appdaemon could still accept the AppDaemon instance, but just to get the dependencies and pass them to the init method.
With this small, but big change, unit testing would be much easier (one can mock the dependencies and focus on the testing of the component functionality)

1 Like

I am not the project owner but I think I can answer these for you.

  1. When AD was developed, Andrew was an experienced programmer but new to Python. So there is probably still a lot of code that is not very “pythonic” but most likely not due to performance reason.

  2. The AD project is definitely open to PR for any improvements but of course have to be approved by the project owner.

We do have a discord group that you’re welcome to join and discuss AD with us if you’d like. AppDaemon (link good for 7-days)

1 Like