London | 26-Jul-SDC | Alex Jamshidi | Sprint 5 | Prep Exercises - #680
Alex-Jamshidi wants to merge 47 commits into
Conversation
…d through functions as arguments - also fixed an if boolean
LonMcGregor
left a comment
There was a problem hiding this comment.
Good work on these tasks. Most of them are good, I have a few questions about some of them
| # Think of the advantages of using methods instead of free functions. Write them down in your notebook. | ||
|
|
||
| # Better readability of code | ||
| # Encapsulation of that function only to class - cleaner and better security. No newline at end of file |
There was a problem hiding this comment.
Could you expand on these explanations or think of any other benefits / drawbacks?
There was a problem hiding this comment.
I have expanded the explanations
| def is_adult(self): | ||
| dob = self.dob | ||
| today = date.today() | ||
| if dob.year > today.year - 18: return False |
There was a problem hiding this comment.
This sequence of if conditions feels a little complicated. Is there anything you could change to improve it / make it more readable / simplify it?
There was a problem hiding this comment.
I couldn't find a way to simplify it with the existing methodology, however I discovered tuples, which allows the comparison of the year/month/day all the the same time using comparison operators (if the year is the same, check month, if that's the same check the day)
| # exercise_10.py:29: error: Unexpected keyword argument "preferred_operating_system" for "Person"; did you mean "preferred_operating_systems"? [call-arg] | ||
| # exercise_10.py:30: error: Unexpected keyword argument "preferred_operating_system" for "Person"; did you mean "preferred_operating_systems"? [call-arg] | ||
|
|
||
| # Fix all of the places that mypy tells you need changing. Make sure the program works as you’d expect. No newline at end of file |
There was a problem hiding this comment.
When I run this I get two empty lists. Is that the expected output?
There was a problem hiding this comment.
Ah woops.
The preferred_operating_systems is a list of strings and the operating system of a laptop is a string, I was doing a like for like comparison == instead of checking if the list includes the string. Updating using python 'in' operator.
| user_operating_system_str = input("Please enter your preferred operating system (options: ARCH, UBUNTU, MACOS, WINDOWS):\n") | ||
|
|
||
| if user_operating_system_str not in OperatingSystem.__members__: | ||
| sys.exit("Error: Operating system should be written in all caps from given options.") |
There was a problem hiding this comment.
Is there any change you could make to the UX to make this more forgiving, rather than requiring the user to write their input in such a strict format?
There was a problem hiding this comment.
Yes, I have converted all inputs into uppercase so that is no longer a requirement to select an operating system
Learners, PR Template
Self checklist
Task code
CYF-1155
Changelist
Exercises from sprint 5 prep completed