database_module #17
No reviewers
skobkin
Labels
No Milestone
No project
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Blocks
#18 Refactor/Optimize transactions
Miroslavsckaya/tg_rss_bot
Reference: Miroslavsckaya/tg_rss_bot#17
Loading…
Reference in New Issue
No description provided.
Delete Branch "database_interaction"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
@ -5,1 +5,4 @@/.venv# Databaseusers_subscribes.dbI think we'll have not only subscribers data in this database, so it's pretty safe to just call it something like
database.db/bot.db/database.sqlite.@ -0,0 +1,36 @@import sqlite3class Database():__instance = NoneI'm not sure if we need Singleton here at all. We'll better handle database connections using direct injections into class constructors other than just calling them in the middle of the class method.
@ -0,0 +9,4 @@return cls.__instancedef __init__(self):self.conn = sqlite3.connect('users_subscribes.db')We'd better have database path passed as an argument of constructor than just hardcode it here.
@ -0,0 +11,4 @@def __init__(self):self.conn = sqlite3.connect('users_subscribes.db')self.cur = self.conn.cursor()self.cur.execute('CREATE TABLE users (id INTEGER, telegram_id NUMERIC, PRIMARY KEY(id))')It'll most likely cause an error on the second run because the table was already created earlier
So we should do one of:
CREATE TABLE IF NOT EXISTS(for now)@ -0,0 +9,4 @@self.cur.execute('CREATE TABLE IF NOT EXISTS feeds_last_items (feed_id INTEGER, link TEXT NOT NULL UNIQUE, title TEXT, description TEXT, date NUMERIC, FOREIGN KEY(feed_id) REFERENCES feeds(id))')def add_user(self, telegram_id):try:Two things concerns me a bit:
trueifINSERTwas successful andfalseif not.1is incomprehensible, it's not even a constant and you're also can't take it as a boolean because it's inversed (truein case of an error is not obvious).strto the function which expectsintthe static analyzer of your IDE will warn you about it even before you finish the code and run it. The same goes for return types.commit()outside oftryblock. It can also cause an exception to be risen, so you should move right after the statements inside thetryblock. This way is you don't forget to fix return values your function will not cause a fail for an entire app.rollback()to roll back everything were made unless you want to mess up somewhere after that when calling anothercommit().UPD: By the way you'll probably want to return inserted user ID from this method (instead of
int/bool) to be able to create relations with other entities/tables after that.@ -0,0 +1,34 @@import sqlite3By the way just
database.pyis enough.Looks good, but here's a couple of things to make better.
@ -0,0 +13,4 @@self.conn.commit()return self.cur.lastrowiddef get_user(self, telegram_id: str) -> int | None:There is a semantic convention about database interaction method naming and behavior.
get*()should return value or throw an exception if nothing is foundfind*()should return value or Null (Noneor empty array/list in case of collections).So this method should either be named
find_user()or throw an exception when user is not found.@ -0,0 +18,4 @@user_id = self.cur.fetchone()if user_id is None:return Nonereturn user_id[0]I think it's most likely
row(ordata) thanuser_id.@ -0,0 +20,4 @@return Nonereturn user_id[0]def add_feed(self, link: str) -> int:If this method doesn't handle user's subscription to the feed than we should have somethig like
def subscribe_user(user_id: int, feed_id: int)anddef unsubscribe_user(user_id: int, feed_id: int).It's better to call this
urlinstead oflink.Things are getting interesting. Looking forward to see this finished 😺
@ -0,0 +5,4 @@self.cur = self.conn.cursor()self.cur.execute('CREATE TABLE IF NOT EXISTS users (id INTEGER, telegram_id NUMERIC NOT NULL UNIQUE, PRIMARY KEY(id))')self.cur.execute('CREATE TABLE IF NOT EXISTS feeds (id INTEGER, url TEXT NOT NULL UNIQUE, PRIMARY KEY(id))')self.cur.execute('CREATE TABLE IF NOT EXISTS subscribes (user_id INTEGER, feed_id INTEGER, FOREIGN KEY(user_id) REFERENCES users(id), FOREIGN KEY(feed_id) REFERENCES feeds(id))')subscriptionsYou can also add unique index on
user_id, feed_idto make it impossible to accidentally subscribe one user to the same feed twice.This index will also be automatically used when selecting feeds for user in the method returning user's feeds.
@ -0,0 +41,4 @@self.cur.execute('SELECT * FROM feeds')return self.cur.fetchall()def get_users_subscribes(self, user_id: int) -> list:subscribesis a verb. "User subscribes to the feed".It's better to use "subscriptions" here.
By the way as you're selecting not subscriptions themselves but the feeds user subscribed to it could be better to call this something like
get_user_feeds()or evenfind_user_feeds().@ -0,0 +21,4 @@return row[0]def add_feed(self, url: str) -> int:self.cur.execute('INSERT INTO feeds (url) VALUES (?)', (url,))It's better to have consistency in the code style.
Here you have trailing comma, but you don't have it in the
subscribe_user()in the list of parameters.So either remove it here, or add it there.
This is not critical in this project in any way. But in real projects such rules are enforced and CI most likely will reject your code if it's not following team guidelines. But you likely will have tools to check if your code is following the rules of the company/team/project.
I just didn't setup anything like that here 🤷 We only have
pylamahere 😄WIP:database_moduleto database_moduleGood enough.