OOP Critter Caretaker program from "Learning Python for the Absolute Beginner"

I'm currently learning programming and Python from Python Programming for the Absolute Beginner, Third Edition. I've been doing the challenges set out at the end of each chapter but I'm never sure if I'm doing what the author wanted or if I'm doing them the best way or worst way.

The task is to:


Create a Critter Farm program by instantiating several Critter objects and keeping track of them through a list. Mimic the Critter Caretaker program, but instead of requiring the user to care for a single critter, require them to care for an entire farm. Each menu choice should allow the user to perform some action for all of the critters (feed all of the critters, play with all of the critters, or listen to all of the critters). To make the program interesting, give each critter random starting hunger and boredom levels.

I've been learning for about 3 weeks now, and this was part of the first OOP chapter. Some feedback and any way I could better my code would be appreciated.

```
import random

class Critter(object):
"""a virtual pet"""
def __init__(self, name, hunger = '', fun = ''):
print("A new critter called", name, "has been created!")
self.name = name
self.hunger = random.randrange(0, 20)
self.fun = random.randrange(0, 20)

def __str__(self):
"""to see there hunger and fun levels"""
return self.name, self.hunger, self.fun

def health(self):
"""check the health of a critter"""
health = self.fun + self.hunger
if health 20:
self.fun == 20
print(self.name, "says: 'That was great fun, thanks for playing with me.'")

def playall(self):
"""play with all the critters"""
self.__pass_time()
self.fun -= 4
if self.fun 20:
self.fun == 20

def __pass_time(self):
"""simulate the passing of time, to make them more hungry and bored"""
self.hunger += 1
self.fun +=1
if self.hunger > 20:

That's very neat and tidy code, and it is obvious what it is doing. A pleasure to read. And, since the critters are so cute I was smiling all the way through! Good work.

I think you've kind of missed the main point of the exercise though, which is "Create a Critter Farm program by instantiating several Critter objects and keeping track of them through a list." Let's walk through it. Instead of this...

#creating the critters, and storing them in the critlist list
dave = Critter('Dave')
sarah = Critter('Sarah')
clair = Critter('Clair')
ashie = Critter('Ashie')
daen = Critter('Daen')
critlist = []
critlist.append(dave.__str__())
critlist.append(sarah.__str__())
critlist.append(clair.__str__())
critlist.append(ashie.__str__())
critlist.append(daen.__str__())

... you should be doing something like this ...

critlist = [Critter('Dave)',
            Critter('Sarah'),
            Critter('Clair'),
            Critter('Ashie'),
            Critter('Daen')]

... and the reason you should be doing that is it makes a lot of other things much easier. For example, this ...

# play with all critter
if answer == '2':
    dave.playall()
    sarah.playall()
    clair.playall()
    ashie.playall()
    daen.playall()
    print("All the critters say: 'Hurrah for our wonderful owner playing"\
          "such a fun group game with us all!!'")

... becomes something like this ...

# play with all critter
if answer == '2':
    for critter in critlist:
        critter.playall()
    print("All the critters say: 'Hurrah for our wonderful owner playing"\
      "such a fun group game with us all!!'")

Which means you don't have to change the code if you add another critter, or rename one, or delete one. Also it is shorter. And the same goes for playing with them, and talking to them.

(It's a little bit more tricky to do the same sort of thing when selecting the name of a Critter you want to interact with, but since you seem to be doing so well I think I'll leave it to you to work out how. Drop a reply here if you want some help with that.)

I suspect also that you don't actually need separate methods in Critter for, for example, play_one() and play_all() as the only difference is whether the critter responds to you. You could just add a parameter to the call like this ...

critter.play(response_required=False)

and default it to True in the arguments list like this:

def play(self,response_required=True):
    """play with one of the critters"""
    self.__pass_time()
    self.fun -= 4
    if self.fun  20:
        self.fun == 20
    if response_required:
        print(self.name, "says: 'That was great fun, thanks for playing with me.'")

There's a small bug in the talking_one() method, as random.randrange(0,5) gives you a number from 0 to 4, and 5 will never happen. I'd suggest using random.choice instead like this:

def talking_one(self):
    """what the critter will say when spoken too, random choice of 6"""
    self.__pass_time()
    print(random.choice(["I like the colour red.",
                         "I am very sleepy today.",
                         "Today is today, i think.",
                         "I eat my shoe.",
                         "Give me your money.",
                         "I can't think of anything else to say, leave me alone."]))

and do the same sort of thing when talking with all of them.

If you do all of that, your code will be much more concise, and much less dependent on repeating the critter names for everything.

One last point. I found myself getting a bit confused over the variable 'fun'. In my head a bigger number for fun suggests there should be more fun, but yours seems to be the other way round. I'd suggest either renaming 'fun' to something like 'boredom' or changing it so bigger numbers = more fun. I know that's a bit tricky given the way it interacts with 'hunger', but a bit more thought about how to represent this would take away what was the only irritant in reading the code.

You're making excellent progress here. Keep up the good work.

StackExchange Code Review Q#157274, answer score: 4