West Midlands | July SDC | Gabriel Deng | Sprint 4 | Prep exercises#39
West Midlands | July SDC | Gabriel Deng | Sprint 4 | Prep exercises#39gai93003 wants to merge 6 commits intoCodeYourFuture:mainfrom
Conversation
|
Your PR description contained template fields which weren't filled in. Check you've ticked everything in the self checklist, and that any sections which prompt you to fill in an answer are either filled in or removed. If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed). |
|
Your PR description contained template fields which weren't filled in. Check you've ticked everything in the self checklist, and that any sections which prompt you to fill in an answer are either filled in or removed. If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed). |
7 similar comments
|
Your PR description contained template fields which weren't filled in. Check you've ticked everything in the self checklist, and that any sections which prompt you to fill in an answer are either filled in or removed. If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed). |
|
Your PR description contained template fields which weren't filled in. Check you've ticked everything in the self checklist, and that any sections which prompt you to fill in an answer are either filled in or removed. If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed). |
|
Your PR description contained template fields which weren't filled in. Check you've ticked everything in the self checklist, and that any sections which prompt you to fill in an answer are either filled in or removed. If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed). |
|
Your PR description contained template fields which weren't filled in. Check you've ticked everything in the self checklist, and that any sections which prompt you to fill in an answer are either filled in or removed. If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed). |
|
Your PR description contained template fields which weren't filled in. Check you've ticked everything in the self checklist, and that any sections which prompt you to fill in an answer are either filled in or removed. If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed). |
|
Your PR description contained template fields which weren't filled in. Check you've ticked everything in the self checklist, and that any sections which prompt you to fill in an answer are either filled in or removed. If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed). |
|
Your PR description contained template fields which weren't filled in. Check you've ticked everything in the self checklist, and that any sections which prompt you to fill in an answer are either filled in or removed. If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed). |
LonMcGregor
left a comment
There was a problem hiding this comment.
Remember to leave a summary of what work you've done in the PR body on github.
You don't need to commit the__pycache__ directory
Good start on this sprint's tasks, I have spotted a few areas where you could improve code further
| @@ -0,0 +1,5 @@ | |||
| def double(n): | |||
| return n * 3 | |||
There was a problem hiding this comment.
Does this function actually double? What change could you make to resolve the issue?
There was a problem hiding this comment.
I think for this function, there are two possible interpretations. Firstly, the function might have been meant to return the triple value of n. In this case, I could the function name from double(n) to triple(n). Secondly, the function might actually been meant to return double the value of n. Meaning I can just change the 3 to 2.
prep exercises/bank_account.py
Outdated
| @@ -0,0 +1,30 @@ | |||
| def open_account(balances, name, amount): | |||
There was a problem hiding this comment.
Part of the bank account task involved making type annotations, can you try that?
There was a problem hiding this comment.
I have just added the type annotations on the functions
There was a problem hiding this comment.
did you try running the code and spotting what the error was?
| self.preferred_operating_system = preferred_operating_system | ||
| self.address = address | ||
|
|
||
| def is_adult(self): |
There was a problem hiding this comment.
What is a benefit or drawback of writing the function this way?
There was a problem hiding this comment.
Writing a method has many benefits, such as it helps in encapsulating logic, as a result, it makes the method reusable in other instances of the class. The problem of these functions is that they become difficult to test in isolation as compared to free functions.
| def is_adult(self): | ||
| today_date = date.today() | ||
| birth_date = date.fromisoformat(self.date_of_birth) | ||
| age = today_date.year - birth_date.year |
There was a problem hiding this comment.
Would this date calculation work all the time? What about if they are one day away from a 18th birthday in the current year?
There was a problem hiding this comment.
This calculation will not work in this scenario because I am only comparing the years. I have changed the functionality to include the days instead of only comparing the years
prep exercises/generic_exercise.py
Outdated
| class Person: | ||
| name: str | ||
| children: List["Person"] | ||
| age: int |
There was a problem hiding this comment.
Can you solve this in a way that it always prints the current age, rather than hard-coding a specific age?
There was a problem hiding this comment.
I have changed the age to date_of_birth and used the function i used previous to calculate the age of the person as long as the birthday is given.
|
Good work, I think this is complete now |
|
Your PR couldn't be matched to an assignment in this module. Please check its title is in the correct format, and that you only have one PR per assignment. If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed). |
1 similar comment
|
Your PR couldn't be matched to an assignment in this module. Please check its title is in the correct format, and that you only have one PR per assignment. If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed). |
Learners, PR Template
Self checklist