-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Solution #1696
base: master
Are you sure you want to change the base?
Solution #1696
Conversation
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 code looks great and meets all the task requirements! 🎉 You've done a fantastic job implementing the Person
class and the create_person_list
function. Just keep in mind the potential KeyError
exceptions that might arise if the data isn't structured as expected. It's always a good idea to ensure that all necessary keys are present and that relationships are correctly defined. Keep up the great work and happy coding! 😊
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
@@ -1,8 +1,25 @@ | |||
class Person: | |||
# write your code here | |||
pass | |||
people = {} |
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.
The people
dictionary is defined as a class attribute. Ensure that this is intended, as it will store all instances of Person
by name across all instances. If this is not the desired behavior, consider using an instance attribute instead.
def create_person_list(people_dicts: list) -> list: | ||
person_list = [] | ||
for person_dict in people_dicts: | ||
person = Person(name=person_dict["name"], age=person_dict["age"]) |
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.
The Person
class constructor is called with name
and age
keys from the dictionary. Ensure that all dictionaries in people_dicts
contain these keys to avoid KeyError
exceptions.
|
||
for person_dict in people_dicts: | ||
name = person_dict["name"] | ||
instance = Person.people[name] |
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.
Accessing Person.people[name]
assumes that every name in people_dicts
has been added to the people
dictionary. Ensure that all names are unique and have been initialized to avoid KeyError
exceptions.
instance.wife = Person.people[person_dict["wife"]] | ||
elif "husband" in person_dict and person_dict["husband"]: | ||
instance.husband = Person.people[person_dict["husband"]] |
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.
The code assumes that the wife
or husband
keys in person_dict
refer to existing Person
instances. Ensure that these relationships are correctly defined in people_dicts
to avoid KeyError
exceptions.
No description provided.