Skip to content

[DRAFT] Refactor database queries#45

Draft
GjjvdBurg wants to merge 1 commit intomasterfrom
feature/database_refactor
Draft

[DRAFT] Refactor database queries#45
GjjvdBurg wants to merge 1 commit intomasterfrom
feature/database_refactor

Conversation

@GjjvdBurg
Copy link
Owner

@GjjvdBurg GjjvdBurg commented Jul 30, 2021

This is an attempt to refactor the database queries. The idea is to have a single Database instance that is passed to the functions instead of the opened sqlite3 cursor, and to implement all necessary queries as methods of the Database instance.

Feedback on the approach suggested here is much appreciated.

@ericthegrey, before I dive into this refactor, what do you think of the proposed structure?

This is an attempt to refactor the database queries. The idea
is to have a single Database instance that is passed to the
functions instead of the opened sqlite3 cursor, and to implement
all necessary queries as methods of the Database instance.
@ericthegrey
Copy link
Collaborator

Hi @GjjvdBurg

That seems like the next logical encapsulation step for the DB peculiarities. A few comments:

  • The connection could have the same lifetime as the Database object itself.
  • Perhaps instead of returning a realized collection of rows, we could return an iterator encapsulating the cursor (In principle, you'd not have to load everything in memory at once -- although the DB driver might just do that. Not sure if it's worth the overhead.)
  • You might use the opportunity to catch sqlite3.Error when working with the DB.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants