Skip to content

General refactoring and restructuring

Dirk HOFFMANN requested to merge feature/General_cleanup into develop

Created by: elbenfreund

This commit provides a major reorganization and refactoring of our main codebase. In general three main objectives are persued in this.

  1. Instead of few extensive functions with intransparent shared state and monkey patching we try to split our codebase into various decoupled objects. The main idea is to split up each individual "widget" into its own proper class and have it handle all but only the logic it is really concerned with. This has several benefits. For one it is far more transparent which parameters are relevant to the widget. Secondly: if we need to have some sort of state it is confined to the widget that actually needs it. Third: All logic happens right where it is relevant. That should make the code much more maintainable as contributors do not have to browse thorugh the entire codebase out of fear that there may be unforeseen sideeffects through monkey patching. Furthermore, we aim to have a clear distinction between public and private interfaces, further incerasing developer confidence about the contracts between objects. This is of particular relevance when it comes to "refreshing/updateting" widgets. Each widget just implements its own "refresh" logic (making things much more accessable) and then gets called once a global "refresh" is triggered.
  2. Dead, obsolete code that was not used at all has been removed. So were unused imports. We also tried to consolidate the allover code style and naming scheme. We did not go all the way with this in order to avoid blowin up the scope of the PR even more and making code review even harder. For now we tried to fix the worst offenders and leave the rest to a second itteration.
  3. We moved each individual widget class into its own seperate file and placed them under the widgets sub-module. This makes for a clear and transparent namespace as well filesystem structure.

As an additional bonus, we started some rudimentary introduction of proper docstrings using the 'jsdoc' syntax (http://usejsdoc.org/).

General implementation notes:

  • Instead of adding more one-off functions to the Stuff sub-module we make heavy use of local functions now. While, javascript being javascript, this looks a bit confusing at first it ensures that functions and implementation details relevant to only a very specific part of the codebase are close by and do not distract developers dealing with entirely different code regions.
  • We make heavy use of the let keyword now (where the old code used var) as in most cases variable declaration was relevant only in a very limited scope and let is more memory efficient in that case. I doubt there is any measureable difference, but it seems like good practive.
  • In general, we tried not to get too carried away with refactoring algorithms and special implementations as well as to limit the scope of the PR but to focus on the general code layout and structure. Instead I made liberal use of [FIXME] tags to indicate parts that warrent revisiting in order to have another look on specific aspects of the code and the way things are done.

For general reference and ease of transition: We renamed our main objects to better reflect what they actually do as well as removed redundant Hamster prefixes:

  • HamsterControler -> Controler
  • HamsterExtension -> PanelWidget
  • HamsterBox -> ->FactsBox``

Attention: We have changed some of previously asyncronous dbus method calls *Remote to their syncronous *Sync version as it seems more fitting as well as less likely to create race conditions. There may however be a risk of freezing the entire shell if those calls time out (which may be an issue in future if the dbus service uses a remote db as persistent storage). We will have to keep an eye on this. We should be ok for now.

Merge request reports

Loading