Solution#1832
Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am approving your solution because all core functional requirements are met: Actor is correctly defined as a dataclass in models.py, and ActorManager in managers.py implements the required CRUD operations with the proper signatures and a working SQLite connection. The all method correctly returns a list of Actor instances (or an empty list when there are no rows), and create, update, and delete behave as expected for the specified primary key operations.
Regarding [CHECKLIST ITEM #4] about plural table names: in this design, table_name is passed in from outside, so your class itself does not enforce or violate the naming convention; just make sure that, when you actually call ActorManager(db_name, table_name), you use plural names (like "actors") whenever the table stores multiple rows. You might also consider adding a brief docstring or comment near __init__ to document this naming expectation for future maintainers, but this is not a blocker for this task. Overall, your structure, separation into models.py and managers.py, and adherence to the requirements look solid—well done.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
| class ActorManager: | ||
| def __init__(self, db_name: str, table_name: str) -> None: | ||
| self._connection = sqlite3.connect(db_name) | ||
| self.table_name = table_name |
There was a problem hiding this comment.
This violates checklist item #4: "If db contains not only one object - name it plural." Right now self.table_name can be any string, including a singular name like 'object'. Consider enforcing or at least documenting that the table name should be plural (e.g. 'actors') when it stores multiple rows.
2 participants
No description provided.