8
\$\begingroup\$ 
#Creates the Animal class. class Animal: def __init__(self, name, age, type_of_animal): self.name = name self.age = age self.animal_type = type_of_animal def bio(self): #Checks if the animal's type begins with a vowel. if self.animal_type[0].lower() in "aeiou": #Checks if the animal is older than 1. if self.age >= 2: print(f"{self.name.capitalize()} is an {self.animal_type.capitalize()} and they are {self.age} years old.") else: print(f"{self.name.capitalize()} is an {self.animal_type.capitalize()} and they are {self.age} year old.") #If it isn't a vowel it's a consonant. else: if self.age >= 2: print(f"{self.name.capitalize()} is a {self.animal_type.capitalize()} and they are {self.age} years old.") else: print(f"{self.name.capitalize()} is a {self.animal_type.capitalize()} and they are {self.age} year old.") #Animals a1 = Animal("milo", 1, "tree frog") a2 = Animal("diego", 2, "anaconda") a3 = Animal("paul", 1, "aardvark") a4 = Animal("pietro", 7, "cheetah") a5 = Animal("bob", 1, "dog") #Animal list animal_list = [a1,a2,a3,a4,a5] #Adds more animals to the list animal_list.append(Animal("kally", 5, "horse")) animal_list.append(Animal("oni", 1, "python")) animal_list.append(Animal("kat", 4, "ocelot")) #Applies .bio() method to each individual animal in the list. for i in range(len(animal_list)): animal_list[i].bio()

This code defines a system for creating animal objects (such as a frog, anaconda, and ocelot) and provides a way to generate a simple bio for each animal based on its type and age. It's a basic example of object-oriented programming (OOP) in Python, where each animal is an object with attributes like name, age, and species. The goal of the code is to create animals, store them in a list, and then display personalized bios for each animal in a user-friendly format. How can I make this more efficient?

\$\endgroup\$

4 Answers 4

Reset to default
9
\$\begingroup\$

Thank you for sharing. There's some good things going on in this code.

constructor

 def __init__(self, name, age, type_of_animal): self.name = name self.age = age self.animal_type = type_of_animal

This is bog standard and perfectly clear, so you can keep it as it is.

The self.foo = foo pattern comes up pretty often, so once you're comfortable with this pattern you might want to use the @dataclass decorator:

from dataclasses import dataclass @dataclass class Animal: name: str age: float type_of_animal: str

DRY

We have four print() statements that are pretty repetitive. It's great that you wrote this out and produced correct results.

But once you have discovered the true needs of the code, there's an opportunity to instrospect, study, go back and DRY up the code so you don't repeat yourself so much.

The pair of detailed output requirements you discovered are

  • "a" / "an" depending on initial vowel
  • "s" plural, depending on age

The simplest approach would be to define temp variables which then appear in a single unified print() statement.

 article = "an" if self.animal_type[0].lower() in "aeiou" else "a" plural = "s" if self.age >= 2 else ""

Another option would be to define a pair of trivial helper functions which accomplish the same thing. Each helper function would return a string, either "a" / "an", or "" / "s".

design of Public API

The .bio() method is evaluated for side effects -- displaying text on stdout. In many situations it would be more convenient to return a str from that method, and let the caller print() the string as needed. This would make it easy to define automated unit tests for your code.

iterating

This certainly works, but it is slightly clunky:

for i in range(len(animal_list)): animal_list[i].bio()

Prefer this more pythonic approach.

for animal in animal_list: animal.bio()
\$\endgroup\$
1
  • \$\begingroup\$ @rdesparbes, thank you. I will fix it up. I only write code in the restricted subset that will lint clean under mypy —strict, so I don’t tend to encounter that. I was attempting to meet the OP where they are, writing in “old” un-annotated code, and then introducing the new notion of type hinting, but I see now I could do it a little better. \$\endgroup\$ Commented Dec 16, 2024 at 14:41
8
\$\begingroup\$

DRY

There is a lot of repeated code in the bio function. You repeat the age check twice and the print 4 times with few modifications. If you create 2 new variables for the 2 types of checks, you only need 1 print:

def bio(self): # Checks if the animal's type begins with a vowel. article = 'an' if self.animal_type[0].lower() in "aeiou" else 'a' # Checks if the animal is older than 1. year = 'years' if self.age >= 2 else 'year' print(f"{self.name.capitalize()} is {article} {self.animal_type.capitalize()} and they are {self.age} {year} old.")

Naming

You did a good job of naming the class and variables, and you adhered to the recommended naming style in the PEP 8 style guide.

However, the function named bio is not very descriptive. I recommend something like print_biography.

Documentation

PEP 8 recommends adding a docstring to describe the classes and functions. You can convert some of the comments to docstrings. For example:

#Creates the Animal class. class Animal:

would be something like:

class Animal: """ A generic animal """

The function docstring could be:

def bio(self): """ Print the biographical data for the animal """

Input checking

I recommend adding checks for the data types. For example, if someone enters -3 for the age, you should report an error. Also check for large values and invalid input like "dog" for the age (in case someone typed data in the wrong order).

Layout

Since you are new to Python, there are some simple development tools that you might not be aware of.

The code uses some inconsistent indentation and other uses of whitespace. The black program can be used to automatically format your code consistently.

The pylint linting tool can also be used to identify formatting and other issues.

\$\endgroup\$
1
  • 4
    \$\begingroup\$ I would say to prefer ruff check over pylint. The pylint tool offered yeoman's service for years, and has done much good in the world. But nowadays? Go with the far quicker ruff. Also, it offers kinder gentler default settings. For example pylint often flags C0114 / C0115 / C0116 for a missing module / class / function docstring. I feel it's better to omit an obvious redundant docstring for a trivial def get_plural(n: float)->str: function, and that's always a matter for human judgement rather than an automated flag. But there are diverse opinions on this. \$\endgroup\$ Commented Dec 13, 2024 at 20:01
8
\$\begingroup\$

In addition to what the others have said, it would make your class more flexible if Animal.bio returns a string rather than printing one itself.

You can still:

for animal in animal_list: print(animal.bio())

But if you want to output that data to a file, for instance, you wouldn't need to write a new "bio" function.

You may also wish to override __str__ to return the biographical string if you feel this is a reasonable default string representation of your object.

 def __str__(self): return self.bio()

Such that now your loop is simply:

for animal in animal_list: print(animal)
\$\endgroup\$
0
7
\$\begingroup\$

The other answers are fairly complete, but one addition that might be helpful, assuming you don't plan on adding any other descriptions of the animal, would be the __str__ method. This basically returns a string representation that you can create of the object when it's used a string, say, when printing it. In your code, you have the .bio() method, which prints the bio to the console.

This requires you to do animal_list[i].bio() to access the animal's bio. In addition, you could just use for animal in animal_list: animal.bio() instead of iterating through a range. Printing this decreases flexibility, and you have to write more.

def __str__(self): #Checks if the animal's type begins with a vowel. if self.animal_type[0].lower() in "aeiou": #Checks if the animal is older than 1. if self.age >= 2: return f"{self.name.capitalize()} is an {self.animal_type.capitalize()} and they are {self.age} years old." else: return f"{self.name.capitalize()} is an {self.animal_type.capitalize()} and they are {self.age} year old." #If it isn't a vowel it's a consonant. else: if self.age >= 2: return f"{self.name.capitalize()} is a {self.animal_type.capitalize()} and they are {self.age} years old." else: return f"{self.name.capitalize()} is a {self.animal_type.capitalize()} and they are {self.age} year old."

Then, you can just do:

for animal in animal_list: print(animal)

Much cleaner, and more flexible.

\$\endgroup\$

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.