-
Notifications
You must be signed in to change notification settings - Fork 3
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
Challenge sum of digits #43
Conversation
Refined sum_of_digits code and tests: added docstrings, improved test cases, and ensured all linting checks passed.
Refined sum_of_digits code and tests: added docstrings, improved test cases, and ensured all linting checks passed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1-input validation: Think about replacing the assert statement used for input validation by an explicite if statement and a valueError. Because assert is not optimize for input validation. It can be disabled in certains environnement
2- Test names: Test names can be more descriptive to reflect the objective of the test. Like test_with_positive_integer instead of test_positive_integer
3- Test method logic: separate logic within test methods into individual methods for clarity and simplicity
This review is just about making the sum_of_digits.py
and its test more clear, simple, understandable and meet all the requirements.
adding "with" in the test names provides a bit more context about what inputs are being tested against, making the test scenarios clearer and more descriptive for people to read it
Replace the assert statement with an explicit if statement and raise a ValueError in the sum_of_digits function
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have made the requested changes. Could you please review them? If you're satisfied, kindly submit. Thanks so much for your help!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job Semira
I appreciate your hard work and dedication.
But it seems some of your ruff tests don't pass. Check again before merging.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your reviews and suggestions were spot-on! Thanks for your feedback!
…te if statement and a valueError
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Félicitations for the great work Semira
Thanks for your review! |
name: solution review
about: A template PR for code review with a checklist
Behavior: Write a function that takes a positive integer and returns the sum of its digits.
Files
/tests/test_file_name.py
Unit Tests
Function Docstring
Raises:
The Function
Strategy
Do's
Don'ts
Implementation
when it's too restricting.
print
statements anywhere