Object-oriented calorie counter

Question

Object-oriented program for a calorie counter sporting polymorphic methods for both genders.

Principles to adhere to:

• Uniform Access Principle
• Polymorphism
• Dependency inversion principle through the Factory Method pattern
• Single responsibility principle

Concerns for future extension:

• BPM curve for exercise acts of longer duration, can this be transparently added?

Other concerns:

• Unit prefixes for names, e.g. weight_kg, exercise_duration_min. Are these bad?
• Overengineered? Yes, I'm not going to argue against that; we don't have more than one method in our classes which eliminates the need to have them at all. Assuming we had more than one method, however, is it still overdone?
• Are any of the principles I claimed to follow not followed or properly followed? Especially thinking of the Factory Method pattern.

By the way, the formula for calculating calories burned doesn't seem to be right, but that doesn't really matter for our purposes.

# oo-calorie_counter.py
from abc import ABC, abstractmethod
def main():
 """Setup and initialization.
 """
 person_attributes = get_person_attributes()
 person = PersonFactory.make_person(person_attributes)
 print("You've burnt approximately {0} calories".format(round(person.calories_burned, 1)))
def get_person_attributes():
 """Prompts a user for their details
 """
 person_attributes = {
 'gender': input('Your gender: '),
 'weight_kg': int(input('Your weight in Kg: ')),
 'age': int(input('Your age: ')),
 'bpm': int(input('Your heart rate: ')),
 'exercise_duration_min': int(input('Your your exercise duration in minutes: '))
 }
 return person_attributes
class PersonFactory:
 @staticmethod
 def make_person(person_attributes):
 """
 Instantiate the proper a person of proper gender based on the provided attributes.
 Args:
 person_attributes: A dictionary containing the keys:
 ('gender', 'age, 'weight_kg', exercise_duration_min, bpm)
 """
 gender = person_attributes['gender']
 del person_attributes['gender']
 if gender.lower() == 'male':
 return Male(**person_attributes)
 elif gender.lower() == 'female':
 return Female(**person_attributes)
 else:
 raise RuntimeError('Unknown gender')
class Person(ABC):
 def __init__(self, weight_kg, bpm, age, exercise_duration_min):
 self.weight_kg = weight_kg
 self.age = age
 self.bpm = bpm
 self.exercise_duration_min = exercise_duration_min
 @property
 @abstractmethod
 def calories_burned(self):
 raise NotImplementedError
class Male(Person):
 @property
 def calories_burned(self):
 return ((self.age * 0.2017) - (self.weight_kg * 0.09036) + (self.bpm * 0.6309) - 55.0969) * self.exercise_duration_min / 4.184
class Female(Person):
 @property
 def calories_burned(self):
 return ((self.age * 0.074) - (self.weight_kg * 0.05741) + (self.bpm * 0.4472) - 20.4022) * self.exercise_duration_min / 4.184
if __name__ == '__main__':
 main()

Answer 1

I like your code. It is easy to understand and pretty straight-forward.

Regarding your question of the measuring units: yes, every variable should have them since you are not operating exclusively on SI units. You have calories (instead of joule) and years and minutes (instead of seconds).

I would just change some of the smaller details, from top to bottom:

• The doc comment for main is wrong. That function not only initializes the program, it is the whole program, including computation and output.

• Rename get_person_attributes to read_person_attributes or input_person_attributes. Functions called get_* usually don't have side-effects and are usually not complicated.

• The gender prompt could be more specific: 'Your gender (male, female): '

• The official spelling for kilogram is kg, not Kg. Although kilo is a large multiplier, it is the only one with a lowercase letter.

• The prompt for the heart rate could include in bpm.

• The Your your is a typo.

• In make_person, the proper a person of proper gender sounds wrong (though I'm not a native English speaker).

• You compute gender.lower() twice, which is unnecessary.

• Do you prefer burned or burnt? Choose one and use it everywhere.

Having each of the complicated formulas in a single line is very good. One might be attempted to split it into several small formulae or extract the factors to a separate class CaloriesCalculatorFactors, but that would make the code harder to understand.

• Martin Kleiven: Your suggestions all make sense, it was sloppy to not proofread anything. Still, the issue of whether there are any fundamental design issues is still open. Although, as you say, "it's straight-forward" that covers it, right?
• Roland Illig: I intentionally didn't comment on the use of design patterns since I don't like most of them. Your use of the Builder pattern is good though, since you just say "I want a person with these attributes". To be precise, you should rather ask for a CaloriesCalculator instead of a Person, but that's close to nitpicking.
• Martin Kleiven: Okay. I just have one burning question left. get_person_attributes have a side-effect where it deletes an element of the dictionary passed as an argument. To what extent is this a bad idea? If at all.
• Roland Illig: For most of your program executions, it will not matter because the caller typically doesn't reuse the person_attributes. On the one hand that's good because your program works as intended. But some day it will break mysteriously when you call the PersonFactory twice with the same argument. The exception will be pretty clear in that case, so it wouldn't be much effort to fix the problem then. But what if the code evolves over time and more and more design patterns are involved, so that the exception is not thrown near its actual cause? Then it can become complicated.
• Richard Neumann: h (hecto) and da (deca) are also unit prefixes > 1 with lower case letters. Just saying.