Make it Concise

Improving code style and making it more concise can often help improve its clarity and reduce complexity, although there are certainly limits.

1) Refactoring Example

For example, consider the following program:

def calc_total(orders, costs):
    """
    Given an orders dictionary mapping item (str) -> quantity (int) and a
    costs dictionary mapping item (str) -> cost (float), find and return
    the total cost of all the orders.

    You may assume that orders and costs contain the same keys, and that all
    the values are non-negative numbers.
    """
    t = []
    i = list(orders)
    j = 0
    while j < len(i):
        k = i[j]
        if orders[k] > 0 and costs[k] > 0:
            t.append(orders[k] * costs[k])
        j += 1
    return sum(t)


print(calc_total({}, {}))
# expected 0
print(calc_total({'carrots': 5, 'strawberries': 1},
                 {'carrots': .1, 'strawberries': 3.25}))
# expected 3.75

One path to refactoring this code may look as follows:

# (removing docstrings and tests for concision)

def calc_total(orders, costs):
    # original code, where do we even begin?
    t = []
    i = list(orders)
    j = 0
    while j < len(i):
        k = i[j]
        if orders[k] > 0 and costs[k] > 0:
            t.append(orders[k] * costs[k])
        j += 1
    return sum(t)


def calc_total(orders, costs):
    # first, let's use some better variable names
    total_costs = []
    items = list(orders)
    i = 0
    while i < len(items):
        item = items[i]
        if orders[item] > 0 and costs[item] > 0:
            total_costs.append(orders[item] * costs[item])
        i += 1
    return sum(total_costs)


def calc_total(orders, costs):
    # second, let's remove some unneeded code
    total_costs = []
    items = list(orders)
    i = 0
    while i < len(items):
        item = items[i]
        # if statement not needed, all values are non-negative!
        total_costs.append(orders[item] * costs[item])
        i += 1
    return sum(total_costs)

def calc_total(orders, costs):
    # third, let's write this in terms of a for loop instead of a while loop
    total_costs = []
    for item in orders:
        total_costs.append(orders[item] * costs[item])
    return sum(total_costs)

def calc_total(orders, costs):
    # final one-line result: write a list comprehension
    return sum([orders[item] * costs[item] for item in orders])

Note how we systematically made small changes (making lots of changes all at once is very prone to bugs!) While refactoring it is a good idea to frequently test your code after each small change to make sure that it still works as expected.

2) Refactoring Practice

To practice some of these skills, the following questions show correct functions written across multiple lines, with potentially bad code style. For each question, rewrite the function with better style. For an extra challenge, you can try writing the function in one line of code (but all correct solutions with meaningful style improvements will also get full points.)

def combine_dicts(orders, costs):
    """
    Given an orders dictionary mapping item (str) -> quantity (int) and a
    costs dictionary mapping item (str) -> cost (float), find and return
    a new dictionary mapping item -> (quantity, cost, total).

    You may assume that orders and costs contain the same keys, and that all
    the values are non-negative numbers.
    """
    t = {}
    i = list(orders)
    j = 0
    while j < len(i):
        k = i[j]
        t[k] = (orders[k], costs[k], orders[k] * costs[k])
        j += 1
    return t

if __name__ == "__main__":
    print(combine_dicts({}, {}))
    # expected: {}
    print(combine_dicts({'carrots': 5, 'strawberries': 1},
                    {'carrots': .1, 'strawberries': 3.25}))
    # expected: {'carrots': (5, 0.1, 0.5), 'strawberries': (1, 3.25, 3.25)}

Paste and submit your refactored combine_dicts function below:

Formatting Help

Loading...

def is_divisible(x, y):
    if x % y == 0:
        return True
    else:
        return False


def get_divisible(x, nums):
    """
    Given an integer x, and a list of integers nums,
    return a new list of nums that are evenly divisible by x.

    You may assume that none of the numbers are 0.
    """
    r = []
    i = -1
    while i < len(nums) - 1:
        i += 1
        if is_divisible(nums[i], x):
            r = r + [nums[i]]
        else:
            r = r

    return r

if __name__ == "__main__":
    print(get_divisible(5, []))
    # expected []
    print(get_divisible(5, list(range(1, 20))))
    # expected [5, 10, 15]
    print(get_divisible(2, [i for i in range(10, 100, 5)]))
    # expected [10, 20, 30, 40, 50, 60, 70, 80, 90]

Paste and submit your refactored get_divisible function below:

Formatting Help

Loading...

Next Exercise: Survey

Back to exercises