Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I'm trying to see if there is a better way to design this. I have a class Animal that is inherited by Male and Female classes (Female class has an additional attribute). I also have a class called Habitat, an instance of each would contain any number of Animals including 0.

class Animal:

    def __init__(self, life_span=20, age=0):
        self.__life_span = life_span
        self.__age = age

    def get_life_span(self):
        return self.__life_span

    def get_age(self):
        return self.__age

    def age(self):
        self.__age += 1


class Female(Animal):

    __gender = 'female'

    def __init__(self, pregnant=False):
        self.__pregnant = pregnant

    def impregnate(self):
        self.__pregnant = True


class Male(Animal):

    __gender = 'male'


class Habitat:

    def __init__(self, list_of_animals=[Male(), Female()]): 
        self.__list_of_animals = list_of_animals

    def __add_male(self):
        self.__list_of_animals.append(Male())

    def __add_female(self):
        self.__list_of_animals.append(Female())

    def add_animal(self):
        if random.choice('mf') == 'm':
            self.__add_male()
        else:
            self.__add_female()    

    def get_population(self):
        return self.__list_of_animals.__len__()

Before I add any more functionality, I would like to find out if that's a proper way to design these classes. Maybe there is a design pattern I can use? I'm new to OOP, any other comments/suggestions are appreciated.

share|improve this question

migrated from stackoverflow.com Feb 11 '13 at 1:28

This question came from our site for professional and enthusiast programmers.

2  
FWIW, don't call obj.__len__(), use len(obj) instead. Also, no need to use name mangling (double leading underscores): Just use self.age or self._age, especially use the former in favor of trivial getters (you can later replace them with a property, if you need that extra power, without changing the way client code works with your objects). –  delnan Feb 10 '13 at 0:40
    
It would appear you're new to OOP in general - what's your experience? –  Jon Clements Feb 10 '13 at 0:41
    
@tallseth: already flagged as such. OP: if the moderators agree, your question will be automatically migrated for you. Please do not double-post. –  Martijn Pieters Feb 10 '13 at 0:41
    
@JonClements somehow I managed to avoid the whole thing and it's been bugging me for years, so now I'm trying to get a grip of it –  dfo Feb 10 '13 at 0:46
    
@MartijnPieters thanks, can I do it myself without waiting for moderators? –  dfo Feb 10 '13 at 0:48

1 Answer 1

In Habitat.__init__, you've committed a classic beginner's fallacy: using a list as a default argument.

In Python, every call to a function with a default argument will use the same default object. If that object is mutable (e.g. a list) any mutations to that object (e.g. .append) will affect that one object. In effect, all your Habitats will end up using the exact same list unless you specify a non-default argument.

Instead, use None as the default argument, and test for it:

def __init__(self, animals=None):
    if animals is None:
        animals = [Male(), Female()]
share|improve this answer
    
Thanks, I fixed that. –  dfo Feb 10 '13 at 23:20

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.