Skip to content

Proposed Code Layout

elbenfreund edited this page Oct 31, 2015 · 4 revisions

In my efforts to hack on hamster I came under the impression that its code is layed out in a rather bizarre way. Most of all, the biggest chunk of backend functionality is mangled into the times overloaded Storage-class.

  • first there is db.db.Storage(), which act as somewhat of a "Controler"-class, effectively calling on now yet defined functions to actually do any of the work and linking those to some basic signal hooks.
  • after that we inherit from this class a first time in storage.db.db.which provides utility functions to actually perform CRUD-operations interacting with sqlite. (which probably could/should be handled by a dedicated library/framework such as sqlalchemy instead of self made queries). By that time thats more than 1.2k LOC barely documented code.
  • and finally we inherit from db.storage.Storage() yet again to build our dbus related glue layer ob top of it which adds roughly another 300 LOC.

Besides the difficulty in reading this, and for new project contributers to understand what functionaluty originates from where this results in difficulties for packaging, testing and validation.

I recomend putting some major work into refactoring this. As quite a bit of functionality is inconsistently found in several stages of the iniheritence-tree I am not sure how nessessary/possible/efective/desired not breaking things is.

My suggestion would be the following:

  • leave db.db.Storage() as it is.
  • Refactor db.storage.Storage() into a proper controler-class for what may be called 'hamsterlib'. That is a package that provides all the actual timetracking functionality in terms of parsing, validation, and CRUD of persistent data. Part of that refactoring could be to not directly work towards a specific storage-implementation (like db.db.Storage) but to provide a more storage-implementation agnostic solution that would allow for an easy and plugable way to change the storage-backend at a later time.
  • Refactor hamster-service so just include and delegate to formentioned Controler class, handle return feedback and provide a dbus interface.
  • Put anything that is on the front end: cli-client, gtk-client etc. into a dedicated packacke. there is no real reason why it should blurr dependencies and package struckture (imho).

As a consequence 'hamsterlib' and 'hamster-service' can and should be packaged seperatly, where 'hamsterlib' would be a dependency for 'hamster-service'.

Please let me know what you think of this. I do not mean to be rude but instead am very grateful for all the work everyone has put into this. And I love using hamster on a daily basis. But now that I looked at its code I feel that some investment in its foundation is warented in order to achive mid and longterm benefits. In its current state I feel it is not realy inviting contribution and may be one of the reasons why hamster as a project failed to attracked a more stable dev-community despite the occational motivated passer by. I would also like to hint again to my suggested dev mailinglist

Clone this wiki locally