Determine the ratio of the contents of a pot to the (metal) volume of of the pot

Question

This is my first Python project, so:

create a pot with dimensions (radius, thickness of wall, thickness of base, height), fill it with food (to a specific height). Determine the ratio between the volume of the food and the volume of the (metal in the) pot.

I am aware that this code does not take handles, rims, dents and other funny stuff into consideration.

import math
pi = math.pi
class Pot:
    """create a pot with contents, determine the ratio between the contents and the pot"""

    def __init__(self,inner_radius,wall_width,base_thickness,height,content_height):
        self.wall_width = wall_width
        self.inner_radius = inner_radius
        self.outer_radius = self.inner_radius + self.wall_width
        self.base_thickness = base_thickness
        self.height = height
        self.content_height = content_height

    def get_cylinder_volume(self,radius,height):
        cylinder_volume = pi * (radius ** 2) * height
        return cylinder_volume

    def get_pot_wall(self):
        inner_cylinder = self.get_cylinder_volume(self.inner_radius,self.height)
        outer_cylinder = self.get_cylinder_volume(self.outer_radius,self.height)
        pot_wall = outer_cylinder - inner_cylinder
        return pot_wall

    def get_pot_base(self):
        pot_base = self.get_cylinder_volume(self.inner_radius,self.base_thickness)
        return pot_base

    def get_pot_volume(self):
        pot_volume = self.get_pot_base() + self.get_pot_wall()
        return pot_volume

    def get_content(self):
        content = self.get_cylinder_volume(self.inner_radius,self.content_height)
        return content

    def get_ratio(self):
        ratio = self.get_content()/self.get_pot_volume()
        return ratio

    def check_ratio(self,desired):
        if (self.get_ratio() < desired):
            print ("too small")
        else:
            print (self.get_ratio())

Answer 1

The only use you have for the math module is pi. You even use pi = math.pi. Well, Python made a shortcut for that:

from math import pi

---

Your __init__ method has some repetition. How can we fix that? The collections module has something we might be looking for: namedtuple(). You can use it like this:

from collections import namedtuple

class Pot(namedtuple('Pot', [
    'inner_radius', 'wall_width', 'base_thickness', 'height',
    'content_height'])):

    def __init__(self, *a):
        self._fields += ('outer_radius',)
        self.outer_radius = self.inner_radius + self.wall_width

    def get_cylinder_volume(self):
    ...

The namedtuple takes care of your arguments for you. Your outer_radius attribute is added to the fields in the first line of the __init__ method. We do that so that it shows up when a Pot instance is printed. It is then assigned to in the second line.

---

In all of your methods, you define a temporary variable to hold the return value and then return that temporary variable. There are, I believe, two reasons to create one-use temporary variables:

• To reduce the number of characters in a line

• To make clear to future readers what this part of the equation is.

In this case, it actually raises the number of characters in the line. That removes the first possibility. As for the second, the name of the function is just as clear as the name of the variable. Therefore, I would just return ... instead of assigning a variable to it then returning the variable.

---

Your naming is good. It is very clear what the logic of your code is. You also follow PEP 8's recommendations on using snake_case for instance variables and function names, and using PascalCase for class names. Congratulations! You pass.

---

check_ratio() does some printing. I wouldn't do that. I think it should return a value, and whatever is calling it can do what it wants. I would create something similar to the Python 2 cmp() function:

def check_ratio(self, desired):
    ratio = self.get_ratio()
    if ratio < desired:
        return -1
    elif ratio > desired:
        return 1
    return 0

Then whatever is calling it can do whatever it wants with that information. That makes the method much more portable. Even if you don't change that, I do have a couple things to say.

You called self.get_ratio() twice. That is unnecessary. It is one of those cases where I would create another variable (as seen above).

You put a space between print and (...) That makes it look like a statement instead of a function. Python 3 does not have print as a statement. In Python 2, use print ...; in Python 3, use print(...). Don't mix the two.

Answer 2

You could get rid of many temporary variables, by returning the result right away:

import math
pi = math.pi
class Pot:
    """create a pot with contents, determine the ratio between the contents and the pot"""

    def __init__(self,inner_radius,wall_width,base_thickness,height,content_height):
        self.wall_width = wall_width
        self.inner_radius = inner_radius
        self.outer_radius = self.inner_radius + self.wall_width
        self.base_thickness = base_thickness
        self.height = height
        self.content_height = content_height

    def get_cylinder_volume(self,radius,height):
        return pi * (radius ** 2) * height

    def get_pot_wall(self):
        inner_cylinder = self.get_cylinder_volume(self.inner_radius,self.height)
        outer_cylinder = self.get_cylinder_volume(self.outer_radius,self.height)
        return outer_cylinder - inner_cylinder

    def get_pot_base(self):
        return self.get_cylinder_volume(self.inner_radius,self.base_thickness)

    def get_pot_volume(self):
        return self.get_pot_base() + self.get_pot_wall()

    def get_content(self):
        return self.get_cylinder_volume(self.inner_radius,self.content_height)

    def get_ratio(self):
        return self.get_content()/self.get_pot_volume()

    def check_ratio(self,desired):
        if (self.get_ratio() < desired):
            print ("too small")
        else:
            print (self.get_ratio())

Also, many of your variables are already determined when you initialize the class. Unless you want to change the shape of the pot on the fly (for this I would create a new instance of Pot with the new dimensions), the only thing I would expect to change is the amount of content in the pot. So, pot_wall, pot_base, pot_volume can all be calculated in init and set to variables of the instance:

def __init__(self,inner_radius,wall_width,base_thickness,height,content_height):
        self.inner_radius = inner_radius
        self.content_height = content_height
        outer_radius = inner_radius + wall_width

        self.pot_volume = self.get_cylinder_volume(inner_radius,base_thickness) + self.get_cylinder_volume(outer_radius,height) - self.get_cylinder_volume(inner_radius,height)

...

def get_ratio(self):
    return self.get_content()/self.pot_volume

This would allow you to get rid of get_pot_wall, get_pot_base, get_pot_volume. Or at least the first two, if this is too much inline calculation for you.