Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generalize interface to databases #18

Open
pylipp opened this issue Oct 10, 2019 · 6 comments
Open

Generalize interface to databases #18

pylipp opened this issue Oct 10, 2019 · 6 comments

Comments

@pylipp
Copy link
Owner

pylipp commented Oct 10, 2019

In the current implementation, a single interface to tinydb exists: financeager.periods.TinyDbPeriod.
For adding different interfaces (e.g. for sqlite, Postgres), the base class should

  • have abstract methods add_entry, remove_entry, update_entry, get_entry, get_entries
  • provide the methods for validating and preprocessing input
@cardinalion
Copy link
Contributor

cardinalion commented Oct 10, 2019

Is this how you write a class with abstract methods?

class DBPeriod(Period, ABC):
    def __init__(self, name=None):
        super.__init__(name=name)
    
    @abstractmethod
    def add_entry(self, table_name=None, **kwargs):
        pass
    @abstractmethod
    def remove_entry(self, eid, table_name=None):
        pass
    @abstractmethod
    def update_entry(self, eid, table_name=None, **kwargs):
        pass
    @abstractmethod
    def get_entry(self, eid, table_name=None):
        pass
    @abstractmethod
    def get_entries(self, filters=None):
        pass

class TinyDbPeriod(DBPeriod):
        pass```

@pylipp
Copy link
Owner Author

pylipp commented Oct 11, 2019

Yes, exactly!
I'd suggest two more things:

  • derive the existing Period class from ABC, instead of introducing a new DBPeriod class
  • provide docstrings ("""Explanation of parameters, functionality, and return values""") instead of passes for the abstract methods. You can move parts of the docstrings of the existing TinyDbPeriod methods.

@cardinalion
Copy link
Contributor

That would be better.
I understand the first thing. As for the second, I feel like I can't just copy and paste. What would be difference between the docstrings of Period class and the docstrings of TinyDbPeriod.

@pylipp
Copy link
Owner Author

pylipp commented Oct 11, 2019

If the docstring contain a detail that is specific to the tinydb implementation, this detail should remain in the TinyDbPeriod. The general parts can go into Period.

@cardinalion
Copy link
Contributor

That makes sense

@pylipp
Copy link
Owner Author

pylipp commented Oct 11, 2019

Did you install the dev tools as described in the section Contributing of the README? This way, formatting of the commit content and the commit message is automatically taken care of.

@pylipp pylipp added this to the v1.0.0 milestone Oct 31, 2019
@pylipp pylipp removed this from the v1.0.0 milestone Jan 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants