Repository navigation
Manchester | 26-SDC-Jul | Fithi Teklom | Sprint 5 | Prep exercises - #681
Fithi-Teklom wants to merge 10 commits into
Conversation
Fithi-Teklom
added
Needs Review
Khantdotcom
left a comment
Khantdotcom
left a comment
There was a problem hiding this comment.
Mandatory fix required : Variable declarations missing in Person class.
|
|
||
|
|
||
| def get_address(person: Person) -> str: | ||
| return person.address |
There was a problem hiding this comment.
Does person have address in your data model?
There was a problem hiding this comment.
Thanks for the feedback. Could you clarify why the variable declarations are a mandatory fix here?
My understanding is that assigning self.name, self.age and self.preferred_operating_system in init already defines the instance attributes and allows mypy to infer their types.
As I understand it, the purpose of this exercise is to demonstrate that mypy can detect access to an attribute, such as address, that isn't defined on the Person class. Since address is intentionally absent from the data model, mypy should flag that access.
There was a problem hiding this comment.
Ohh yes you're right. I overlooked it.
For more practice on attr access, check out those methods :
• getattr(obj, name[, default])
• setattr(obj, name, value)
• hasattr(obj, name)
• delattr(obj, name).
Fithi-Teklom
changed the title
LonMcGregor
left a comment
LonMcGregor
left a comment
There was a problem hiding this comment.
Good work, Apologies for the late review. I've left a few extra comments
|
|
||
| def is_adult(self): | ||
| today = date.today() | ||
| age = today.year - self.date_of_birth.year |
There was a problem hiding this comment.
Will this always work? What if today is the 10th of October and the person was born on the 20th of october 18 years ago?
There was a problem hiding this comment.
Thank you, for the feedback. I have improved the calculation.
| class Person: | ||
| name: str | ||
| age: int | ||
| children: list |
There was a problem hiding this comment.
This list could contain anything, is there a way of using generics to force the right type of list?
There was a problem hiding this comment.
Thanks for the feedback! I've changed the annotation from children: list to children: list["Person"] so that the type checker knows the list should contain Person instances rather than arbitrary values.
| # │ | ||
| # ├── get_name() | ||
| # │ | ||
| # ▼ |
LonMcGregor
added
Reviewed
Fithi-Teklom
added
Needs Review
Self checklist
Task code
CYF-1155
Changelist
Completed the sprint 5 prep exercises covering types, classes and objects, dataclasses, generics, enums , methods and inheritance.