4
\$\begingroup\$

The idea is to accept an English string of math operations and evaluate it from left to right.

Regarding Order of Operations: when speaking operations aloud, it is often with implied parenthesis that denotes a left-to-right chain.

For example:

4 plus 3 times 2 squared === (((4 plus 3) times 2) squared)

The rhythm which it was spoken dictates whether or not to square the 2 before multiplying — but in this case the goal is to go strictly left-to-right.

The starting point to my review here was my answer to a StackOverflow question which was direct, correct, but unsatisfying in design and style.

import math
import re
import operator

# try "3 add 4 times 5"
inputs = input(">").lower()

# Using RE to find all numbers in string.
numInString = [int(d) for d in re.findall(r'-?\d+', inputs)]
inputs = inputs.split()

print('Numbers found: ', numInString)
print('Inputs found: ', inputs)

# Define all of the operators
def multiplication():
    multAns = operator.mul(numInString.pop(0) , numInString.pop(0))
    numInString.insert(0, multAns)
    print(multAns)

def division():
    divAns = operator.truediv(numInString.pop(0) , numInString.pop(0))
    numInString.insert(0, divAns)
    print(divAns)

def addition():
    addAns = operator.add(numInString.pop(0) , numInString.pop(0))
    numInString.insert(0, addAns)
    print(addAns)

def subtraction():
    subAns = operator.sub(numInString.pop(0) , numInString.pop(0))
    numInString.insert(0, subAns)
    print(subAns)

def squareRoot():
    SqrtAns = math.sqrt(numInString.pop(0))
    numInString.insert(0, SqrtAns)
    print(SqrtAns)

def squared():
    SquareAns = math.pow(numInString.pop(0), 2)
    numInString.insert(0, SquareAns)
    print(SquareAns)

def cubed():
    CubedAns = math.pow(numInString.pop(0), 3)
    numInString.insert(0, CubedAns)
    print(CubedAns)

def power():
    PowerAns = math.pow(numInString.pop(0), numInString[1])
    numInString.insert(0, PowerAns)
    print(PowerAns)

# Dictionary mapping search words to operator function
Operation = {
    'multiply': multiplication,
    'times' : multiplication,
    'multiplied': multiplication,
    'divide': division,
    'divided': division,
    'into': division,
    'add': addition,
    'sum': addition,
    'added': addition,
    'subtract': subtraction,
    'minus': subtraction,
    'take': subtraction,
    'subtracted': subtraction,
    }

for words in inputs:
    if words in Operation:
        print(Operation[words]())
  1. How should I be naming variables? Are Operation and numInString good names?

  2. Should I print the result of a function operator?

  3. Should I be storing the numbers in a global list (e.g. numInString) instead of passing them to operator functions?

  4. Should I print errors like this?:

    print('NO OPERATION GIVEN')
    
  5. Is it okay to assume all numbers will be of type int?

  6. Should I leave unguarded script code such as input('>')?

\$\endgroup\$

1 Answer 1

7
\$\begingroup\$
  1. How should I be naming variables? Operation and numInString good names?

Great question, Python has an extensive but approachable style guide call PEP8. Specifically local variables should be lowercase, Global Constants should be UPPERCASE, and Global variables should be avoided.

  1. Should I print the result of a function operator?

In order to use it later you should return the value:

Instead of: print(multAns) do return multAns

  1. Should I be storing the numbers in a global list, numInString, instead of passing them to operator functions?

No, pass the numbers to the operator and save the returned result

result = oper(a=1, b=3)

instead of

a = 1
b = 3
oper()
  1. Should I print errors like this?:

    print('NO OPERATION GIVEN')
    

No, raise errors when you come to this sort of situation.

ValueError("Cannot match an operator to \"{}\"".format(inputstring))
  1. Is it ok to assume all numbers will be int?

It would be better to handle the case of decimal number, especially if you want to be able to do true division (which was in your code)

My revision with plenty of comments, incorporating my above suggestions

import re
from decimal import Decimal

Dictionary mapping search words to operator function

operations = {
    '<a> (times|multiplied by) <b>':
        lambda a, b: a * b,
    '<a> divided by <b>':
        lambda a, b: a / b,
    '(add|sum)? <a> (and|plus) <b>':
        lambda a, b: a + b,
    'subtract <a> and <b>':
        lambda a, b: a - b,
    '<a> (minus|take) <b>':
        lambda a, b: a - b,
    '<a> squared':
        lambda a: a**2,
    '<a> cubed':
        lambda a: a**3,
    '<a> raised to the power <b>':
        lambda a, b: a**b,
    }

think of lambda as "make_function"

def plus(a, b):
    return a + b

is the same as

plus = lambda a, b: a + b

also you can use the actual operator: eg. * instead of operator.mul + instead of add

Match any number eg. 8, 8.8, -3.14 https://stackoverflow.com/questions/15814592/how-do-i-include-negative-decimal-numbers-in-this-regular-expression this is defined as a Global Constant

NUM_RE = "-?\d+(\.\d+)?"

Match that number and give it a name that can be access later via m.groupdict()

def named_number_regex(name):
    return "(?P<{name}>{num})".format(name=name, num=NUM_RE)

variable_names = ['a', 'b']
for name in variable_names:
    # replace all of the name placeholders with
    # the actual regex for matching a named number
    #     eg. "<a>" ---> "(?P<a>-?\d+(\.\d+)?)"
    # placeholder were used just to make the above patterns easier to read
    old = '<{}>'.format(name)
    new = named_number_regex(name)
    operations = {k.replace(old, new): v for k, v in operations.items()}

# allow for varying amount of whitespace
operations = {'\s*' + k.replace(' ', '\s*'): v for k, v in operations.items()}

Now the patterns look like this:

#
# \s*(?P<a>-?\d+(\.\d+)?)\s*squared
#
# let's break it down:
#
# \s*                                 ignore leading whitespace
#    (?P<a>-?\d+(\.\d+)?)             match any number, name it 'a'
#                        \s*          ignore other whitespace
#                           squared   only match if you find " squared"

Function to take a string and and recursively simplify it until we either get a number or encounter an error.

def simplify(inputstring):
    print(inputstring) # see what is going on internally

    # if you get down to a single number return its value
    if re.fullmatch(NUM_RE, inputstring) is not None:
        return Decimal(inputstring)

    for matcher, oper in operations.items():
        # iterate over all key: value pairs in the dict "operations" eg:
        #    matcher = '(?P<a>\d+) (times|multiplied by) (?P<b>\d+)'
        #    oper = lambda a, b: operator.mul(a, b)
        m = re.match(matcher, inputstring)
        if m is None:
            continue

        # dict of named matches eg {'a': '4', 'b': '8'}
        numbers = m.groupdict()

        # convert text eg. '4' strings into integers eg. 4
        # this is like your [int(d) for d in re.findall(r'-?\d+', inputs)]
        # Decimal handles ints and floats, with floating point rounding errors
        numbers = {k:Decimal(v) for k, v in numbers.items()}

        # now numbers = {'a': 4, 'b': 8}

        # get the result of the matched operation
        # Note: ** means call function with keyword args from a dict
        #     eg. oper(a=4, b=8)
        result = oper(**numbers)

        # substitute the result of oper with the matched string
        # Note we can lose precision here when casting Decimal to str
        simplified = re.sub(matcher, str(result), inputstring)

        # now simplify it further if possible
        # this is call recursion, it will continue to simplify until we get down to just a number
        print('= ', sep='', end='')
        return simplify(simplified)

    raise ValueError("Cannot match an operator to \"{}\"".format(inputstring))

Test Cases

def test_a_few_cases():
    print("\nSimple Addition:")
    result = simplify("4 plus 8")

    print("\nDemonstrate the recursion:")
    result = simplify("2 squared squared squared squared")

    print("\nMore elaborate:")
    result2 = simplify("2 plus 1.11 times -7 raised to the power 6")

    print("\nDemonstrate an Error:")
    result = simplify("2 squared squared squared squard")

def main():
    print("\nYour input:")
    yourresult = simplify(input(">").lower())

if __name__ == '__main__':
    test_a_few_cases()
    main()
\$\endgroup\$
2
  • \$\begingroup\$ You don't seem to have defined num, and your code doesn't take order of operations into account. \$\endgroup\$ Commented Dec 11, 2017 at 23:11
  • \$\begingroup\$ Thanks, num somehow got left over from an older revision. Previous I used num as here: stackoverflow.com/a/379966/625444. I later decided on Decimal to avoid rounding errors. \$\endgroup\$
    – Zak
    Commented Dec 11, 2017 at 23:15

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.