"Alien" challenge

Question

I have quickly written some code for a task in which takes a name of an 'earthling', their age and then applies a random operator with a random value in between 0 and 100. If their new age is below 0 or above 120, they die. The operator, name, previous and new age must be printed to the screen.

Code:

import operator
import random

ops_table = { "+": operator.add, "-": operator.sub, "*": operator.mul }

def get_earthlings():
    earthlings = [[], []]
    print("Disclaimer: Input XXX as a name to end the naming loop.")

    while True:
        name = input("Please enter a name: ")
        if name == "XXX":
            break

        while True:
            try:
                age = int(input("What is the age of " + name + "? "))

                if age <= 120 and age >= 0:
                    break
                else:
                    print("Please enter an age between 0 and 120.")
            except ValueError:
                print("Please input a valid integer. Try again.")

        earthlings[0].append(name)
        earthlings[1].append(age)
    return earthlings

def random_op():
    return ops_table[random.choice(list(ops_table))]

def is_dead(age):
    if age > 120:
        return True
    return False

def get_op_name(operator):
    for name, func in ops_table.items():
        if func == operator:
            return name

def main():
    earthlings = get_earthlings()

    for index, earthling in enumerate(earthlings[0]):
        operator = random_op()
        random_value = random.randint(0, 100)
        new_age = operator(earthlings[1][index], random_value)
        if is_dead(new_age):
            print(earthling, "at previous age", earthlings[1][index], "(new age", str(new_age) + ") had operator '" + get_op_name(operator) + "' applied with paramater", random_value, "and died.")
        else:
            print(earthling, "at previous age", earthlings[1][index], "(new age", str(new_age) + ") had operator '" + get_op_name(operator) + "' applied with paramater", random_value, "and lived.")

if __name__ == "__main__":
    main()

Is this code efficient? How can I improve it (other than adding comments)?

Answer 1

First, you code is actually bugged. isDead isn't checking if the age is less than zero. It's also more complicated than it needs to be. You can just return the result of the condition directly. It would be better written as:

def is_dead(age):
    return age < 0 or age > 120
    # Or return not (0 <= age <= 120)

and along the same lines, the age check in get_earthlings could be reduced to if 0 <= age <= 120:.

---

Storing the names and ages as two separate lists, nested inside another list is needlessly confusing. This is an example of maintaining "parallel" lists to maintain data, and most of the time, it should be avoided. What if you remove data from one list and forget to remove it from the other? Now you have weird bugs.

This is the perfect case for a class with age and name fields:

class Earthling:
    def __init__(self, name, age):
        self.name = name
        self.age = age

    # A simple Higher Order helper method
    # Not really necessary, but a good example
    def change_age(self, f, other_num): # f is an operator
        self.age = f(self.age, other_num)

    # I flipped the check because it works out a little nicer
    def is_alive(self):
        return 0 <= self.age <= 120

Then, use it like:

e = Earthling("Name", 10)
e.change_age(operator.sub, 20)
print(e.age) # Prints -10
print(e.is_alive()) # False

Or, just have a single list of tuples where the first element in the tuple is the name, and the second is the age. Just keep the data grouped together when they're a part of the same conceptual object.

---

if is_dead(new_age):
    print(earthling, "at previous age", earthlings[1][index], "(new age", str(new_age) + ") had operator '" + get_op_name(operator) + "' applied with paramater", random_value, "and died.")
else:
    print(earthling, "at previous age", earthlings[1][index], "(new age", str(new_age) + ") had operator '" + get_op_name(operator) + "' applied with paramater", random_value, "and lived.")

These lines contain lots of duplication. Whenever you see code that's identical like this in multiple places, stop and think if you can make the code simpler. If you ever need to update the code in the future, you have multiple places to change, and that's not ideal.

In this case, it's actually very easy to fix this up:

# Pick the status here
status_str = "lived" if e.is_alive() else "died"

# Then plug it in here
print(e.name, "at previous age", previous_age, "(new age", str(new_age) + ") had operator '"
      + get_op_name(operator) + "' applied with paramater", random_value, "and", status_str)

I'm picking what the status to print is ahead of time since that's the only part that changes in the two cases, then I just plug that in at the end.

Unfortunately, with the change to classes, this messes up maintaining the previous age. You could do something like old_age = e.age before changing it, or even save the old age in e; although the latter seems a little smelly.

---

get_op_name is pretty inefficient, but it doesn't really need to be efficient. That function will likely never be a performance concern, but if it ever was, you could using some form of "two-way" dictionary idea. Either maintain two dictionaries, or store both "directions" of translations in a single dictionary. Both ways suffer from the downside of having data duplicated, but since the data likely won't change during the operation of the program, that shouldn't be a big deal.

---

For picking the random operation, I find this to be a little neater:

random.choice(list(ops_table.values()))

Unfortunately, values doesn't return a sequence, so it needs to be converted into a list (or another sequence type) before being given to choice.

Answer 2

There are just two minor points that I would like to add on top of @Carcigenicate's answer.

---

In the answer, a class was introduced as possible way to structure the code. While not limited to classes, practice has proven that at least a short bit of documentation can often work wonders. As you may know, Python has an official Style Guide, which also has some recommendations on function/method documentation. It recommendeds to have at least a short oneliner in """triple quotes""" immediately after the function definition.

class Earthling:
    def __init__(self, name, age):
        self.name = name
        self.age = age

    def change_age(self, f, age_update):
        """combine the earthlings age with the update using the given operator f"""
        self.age = f(self.age, age_update)

    def is_alive(self):
        """Check if the earthling is still alive, i.e. between 0 and 120 years old"""
        return 0 <= self.age <= 120

Though it might seem overkill in this simple example, I find it a reasonable exercise to try to convey the gist of the method in question in a few words. This may also help to make sure that your functions are not to complex. Of course you are not limited to a single line and can also write longer documentation strings. To quote from the style guide:

"""Return a foobang

Optional plotz says to frobnicate the bizbaz first.
"""

---

Another pet peeve of mine is string formatting. You were advised to use

print(e.name, "at previous age", previous_age, "(new age", str(new_age) + ") had operator '"
      + get_op_name(operator) + "' applied with paramater", random_value, "and", status_str)

which closely follows your original use of print. I personally don't like the implicit whitespace put between the arguments of print when used this way. I prefer to use explicit string formatting. Since you have not indicated the Python version in use, I will present you two variants. The first way may only be used in Python 3.6 (see What's new in Python 3.6?) and higher, and looks as follows:

print(f"{e.name} at previous age {previous_age} (new age {new_age}) had operator"
      f"'{get_op_name(operator)}' applied with paramater {random_value} and {status_str}")

These are called f-strings for short.

The second and older way which works in Python 2.7 and all of the versions of Python 3 I'm aware of uses .format(...):

print(
    "{} at previous age {} (new age {}) had operator '{}' applied with paramater {} and {}".format(
        e.name, previous_age, new_age, get_op_name(operator), random_value, status_str)
)

By the look and feel I prefer f-strings, though you sometimes just cannot have them if you have to be backwards compatible with Python 2.7.

You can read more about the topic at this blog post, which has a nice comparison of the different ways strings can be formatted in Python.