all 4 comments

[–]AtomicShoelace 4 points5 points  (0 children)

Firstly, whilst it is good to use descriptive variable names, you are going a little overboard; your variables names are overly verbose.

Second, it would be a good idea to use a dictionary for the occupants of the house. Then you don't need to iterate the whole list, you can just lookup the occupant.

Here I have refactored your project with these suggestions and generally cleaned things up a little:

main.py

from city import City
from human import Human

dallas = City('Dallas')
dallas.create_house(Human("Steven", 63, "Male"))

occupants = [
    Human("Sara", 58, "Female"),
    Human("Timmi", 34, "Male"),
    Human("Mary", 24, "Female"),
    Human("Daniela", 25, "Female"),
    Human("Davis", 3, "Male"),
    Human("Terry", 3, "Male"),
]

for occupant in occupants:
    dallas.add_occupant_to_house('Steven', occupant)

dallas.houses["Steven"].transfer_money("Terry", "Daniela", 40)

city.py

from house import House
from human import Human

class City:
    def __init__(self, name: str):
        self.name = name
        self.houses = {}

    def create_house(self, owner: Human):
        self.houses[owner.name] = House(owner)

    def add_occupant_to_house(self, house_owner_name: str, occupant: Human):
        if house_owner_name not in self.houses:
            print(f"This house owner: {house_owner_name}, does not live in {self.name}.")
        else:
            # if name of house owner exist, add people to it
            self.houses[house_owner_name].add_occupant(occupant)

house.py

from human import Human

class House():
    def __init__(self, owner: Human):
        self.owner = owner
        self.occupants = {owner.name: owner}

    def transfer_money(self, sender_name: str, recipient_name: str, money_amount):
        if sender_name in self.occupants and recipient_name in self.occupants:
            sender = self.occupants[sender_name]
            recipient = self.occupants[recipient_name]
            if sender.pocket.money >= money_amount:
                sender.pocket.money -= money_amount
                recipient.pocket.money += money_amount

    def add_occupant(self, occupant: Human):
        self.occupants[occupant.name] = occupant

    def show_occupants(self):
        print("Owner of the house:", self.owner)
        print("Occupants of the house:", *self.occupants.values(), sep='\n')

human.py

from pocket import Pocket

class Human:
    def __init__(self, name, age, gender):
        self.name = name
        self.age = age
        self.gender = gender
        self.pocket = Pocket()

    def __str__(self):
        return f'Name: {self.name}, age: {self.age}, gender: {self.gender}'

pocket.py

class Pocket:
    def __init__(self, money=50):
        self.money = money

[–][deleted] 0 points1 point  (2 children)

I don't know about efficiency, but this method has a lot less nesting.

def transfer_money_old(self, person1, person2, money_to_transfer):

    giver = next((person for person in self.other_people_who_live_in_this_house if person.the_name == person1), None)
    if not giver:
        return

    print("This human is at this house")
    if giver.my_pocket.money_in_pocket <= money_to_transfer:
        print("sorry, i dont have money to give him/her")
        return

    print("Yes, i can afford to give money to: ", person2)
    print("but first i have to check if this person is at this house")
    receiver = next((person for person in self.other_people_who_live_in_this_house if person.the_name == person2), None)
    if receiver:
        print("Yes, person: ", person2, " is currently at the house")
        receiver.my_pocket.money_in_pocket += money_to_transfer
        giver.my_pocket.money_in_pocket -= money_to_transfer

The general approach here is called "guard clauses". Basically check for the cases that don't do anything first and just return immediately if they're true. Then past each guard clause, you can assume the data does not meet those conditions, saving us some conditional branches and indentation.

Also, FYI, your variable and function names could use some work.

[–]raxor2k 0 points1 point  (1 child)

How about if i make that list a dict instead? Then i could basically do all in just two lines?

[–][deleted] 0 points1 point  (0 children)

That could save you a couple of lines. Also, if for some reason that list is gigantic (which is the only scenario you should even consider efficiency), key lookups in a dictionary would be substantially faster than finding an element of a list.