-
Notifications
You must be signed in to change notification settings - Fork 449
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
Improvement: Refactor code to use dict.get() method #1065
Comments
hey @codesankalp can i work on this ? |
Sure @harsh-9in 😄. Assigning you! |
@codesankalp hey was this issue update discussed anywhere? |
@vj-codes This is the referenced comment: anitab-org/stem-diverse-tv#146 (comment) |
@vj-codes thank you for confirming it! I saw the issue being created on stem diverse and thought it could be a little improvement here :) cc @codesankalp |
@harsh-9in any updates on this? |
@harsh-9in if you have any questions about the issue or setting up the project, you can ask us here in comments or on zulip too :) |
@vj-codes Can I work on this issue? |
@codesankalp I don't get this? Why would we need a default value here? The data that comes from the user is already validated and needs to have these keys. |
Yes, it is but when you see the code there is a lot of repetition of code for validation which can be reduced using In the description, I just gave the example of dao/user.py but what I want to convey is to use dict.get method wherever necessary so that code doesn't look repetitive. Example: mentorship-backend/app/api/dao/user.py Lines 244 to 248 in 25d8d99
This can be written as: This needs to be changed in the whole code. |
@vj-codes @epicadk @codesankalp I have created a PR. Please check, if any changes are required. |
Description
As a developer,
I need code to be consistent and error-free.
Mocks
mentorship-backend/app/api/dao/user.py
Lines 42 to 46 in d52d777
For example this can be replaced by the get method i.e.
data.get("KEY", "DEFAULT_VALUE")
to avoid KeyErrorAcceptance Criteria
Update [Required]
Definition of Done
Estimation
2 hours
The text was updated successfully, but these errors were encountered: