Skip to content
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

Week09 bwan1 #67

Open
wants to merge 3 commits into
base: week09
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions refactoring/initial_global.py
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
def average_age():
def average_age(group):
"""Compute the average age of the group's members."""
all_ages = [person["age"] for person in group.values()]
return sum(all_ages) / len(group)


def forget(person1, person2):
def forget(group, person1, person2):
"""Remove the connection between two people."""
group[person1]["relations"].pop(person2, None)
group[person2]["relations"].pop(person1, None)


def add_person(name, age, job, relations):
def add_person(group, name, age, job, relations):
"""Add a new person with the given characteristics to the group."""
new_person = {
"age": age,
Expand Down Expand Up @@ -50,12 +50,12 @@ def add_person(name, age, job, relations):
"Zalika": "landlord"
}

add_person("Nash", 34, "chef", nash_relations)
add_person(group, "Nash", 34, "chef", nash_relations)

forget("Nash", "John")
forget(group, "Nash", "John")

if __name__ == "__main__":
assert len(group) == 4, "Group should have 4 members"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉
you could also only define the group within the if name == "main" block, so that you don't have a group defined if you import this file

assert average_age() == 28.75, "Average age of the group is incorrect!"
assert average_age(group) == 28.75, "Average age of the group is incorrect!"
assert len(group["Nash"]["relations"]) == 1, "Nash should only have one relation"
print("All assertions have passed!")
11 changes: 10 additions & 1 deletion refactoring/initial_person_class.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ def add_connection(self, person, relation):

def forget(self, person):
"""Removes any connections to a person"""
pass
if person in self.connections:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like that you've made sure that they exist, in our example we've thrown a specific exception here but this is also another sensible way to deal with it

del self.connections[person]


def average_age(group):
Expand All @@ -28,10 +29,18 @@ def average_age(group):
if __name__ == "__main__":
# ...then create the group members one by one...
jill = Person("Jill", 26, "biologist")
zalika = Person("Zalika", 28, "artist")
john = Person("John", 27, "writer")
nash = Person("Nash", 34, "chef")

# ...then add the connections one by one...
# Note: this will fail from here if the person objects aren't created
jill.add_connection(zalika, "friend")
jill.add_connection(john, "partner")
zalika.add_connection(jill, "friend")
john.add_connection(jill, "partner")
nash.add_connection(john, "cousin")
nash.add_connection(zalika, "landlord")

# ... then forget Nash and John's connection
nash.forget(john)
Expand Down
31 changes: 27 additions & 4 deletions refactoring/initial_two_classes.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ def __init__(self):

def size(self):
"""Return how many people are in the group."""
pass
return len(self.members)

def contains(self, name):
"""Check whether the group contains a person with the given name.
Expand All @@ -32,17 +32,32 @@ def add_person(self, name, age, job):

def number_of_connections(self, name):
"""Find the number of connections that a person in the group has"""
pass
if name in self.connections:
Copy link

@stefpiatek stefpiatek Dec 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh interesting, you've used a different data structure than our example I think, we've used this in our example

Suggested change
if name in self.connections:
if self.contains(name):

return len(self.connections[name])
else:
return 0
Comment on lines +37 to +38

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is certainly one way to deal with it, the other would be to throw an exception


def connect(self, name1, name2, relation, reciprocal=True):
"""Connect two given people in a particular way.
Optional reciprocal: If true, will add the relationship from name2 to name 1 as well
"""
pass
if self.number_of_connections(name1) != 0:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also could have a guard clause here to check that we know about these people exist in our group

Suggested change
if self.number_of_connections(name1) != 0:
if not self.contains(name1):
raise ValueError(f"I don't know who {name1} is.")
if not self.contains(name2):
raise ValueError(f"I don't know who {name2} is.")
if self.number_of_connections(name1) != 0:

self.connections[name1] += [(name2, relation)]
else:
self.connections[name1] = [(name2, relation)]
if reciprocal:
self.connect(name2, name1, relation, False)

def forget(self, name1, name2):
"""Remove the connection between two people."""
pass
if self.number_of_connections(name1) != 0:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could also add a guard clause here too

for connection in self.connections[name1]:
if connection[0] == name2:
self.connections[name1].remove(connection)
if self.number_of_connections(name2) != 0:
for connection in self.connections[name2]:
if connection[0] == name1:
self.connections[name2].remove(connection)

def average_age(self):
"""Compute the average age of the group's members."""
Expand All @@ -55,8 +70,16 @@ def average_age(self):
my_group = Group()
# ...then add the group members one by one...
my_group.add_person("Jill", 26, "biologist")
my_group.add_person("Zalika", 28, "artist")
my_group.add_person("John", 27, "writer")
my_group.add_person("Nash", 34, "chef")

# ...then their connections
my_group.connect("Jill", "Zalika", "friend")
my_group.connect("Jill", "John", "partner")
my_group.connect("Nash", "John", "cousin")
my_group.connect("Nash", "Zalika", "landlord", False)

# ... then forget Nash and John's connection
my_group.forget("Nash", "John")

Expand Down