0

I am trying to write a basic s-expression calculator in Python using s-expression which can contains add or multiply or both or none or just an integar number.

I tried the following snippet:

def calc(expr):
    print(expression[0])
    if isinstance(expr, int):
        return expr
    elif expr[0] == '+':
        return calc(expr[1]) + calc(expr[2])
    elif expr[0] == '*':
        return calc(expr[1]) * calc(expr[2])
    else:
        raise ValueError("Unknown operator: %s" % expr[0])

# Example usage
# expression = ('+', ('*', 3, 4), 5)
expression = (7)
result = calc(expression)
print(result)

When I tried to pass the expression ('+', ('*', 3, 4), 5) , it gives the correct answer but when I just try to use number 7 or 7 inside tuple (7), it gives the above error. How to solve this?

Reactoo
  • 916
  • 2
  • 12
  • 40
  • 2
    remove `print(expression[0])` – Remi Cuingnet Feb 09 '23 at 08:44
  • Or place it after the branch that exclude the int type. – Dorian Turba Feb 09 '23 at 08:56
  • You get an error from `7` because it isn't an s-expression - you are using tuples to represent your s-expressions, and `7` is an integer, not a tuple. You also get an error from `(7)` because that is **still** not a tuple - it **means the same thing** as `7` (they're just ordinary parentheses like in math). To make that tuple, a trailing comma is needed: `(7,)`. I linked this as a duplicate of the canonical about making such tuples, because that is the underlying problem here. – Karl Knechtel Feb 09 '23 at 09:20
  • Ah, I see. You intend to handle `int`s specially (since there's recursion anyway). There are multiple issues with the debugging `print`: first, it's accessing the global `expression` instead of the passed-in `expr`, and second it's trying to access an element of the tuple before verifying that there is actually a tuple. I think both of these qualify as typos. – Karl Knechtel Feb 09 '23 at 09:25

1 Answers1

0

Code is fine, debug is not

You print used to debug is not correctly placed, or assume expression to be a Sequence, not an int.

  1. [Good practice] Don't print a global variable but local: print(expr). This is less confusing and will help you for debugging this code.

  2. [Branch simplification] Replace every elif with if. Since you return in every branch, you don't need elif. Remove the else too. This will allows you to place code after the first if that will be run for all remaining branches without having to place it in every elif.

def calc(expr: int | tuple):
    print(expr)
    if isinstance(expr, int):
        return expr
    if expr[0] == '+':
        return calc(expr[1]) + calc(expr[2])
    if expr[0] == '*':
        return calc(expr[1]) * calc(expr[2])
    raise ValueError("Unknown operator: %s" % expr[0])
  1. [Fix] Move the print below the first if.
def calc(expr: int | tuple):
    if isinstance(expr, int):
        return expr
    print(expr)
    if expr[0] == '+':
        return calc(expr[1]) + calc(expr[2])
    if expr[0] == '*':
        return calc(expr[1]) * calc(expr[2])
    raise ValueError("Unknown operator: %s" % expr[0])

This code works, both for (7) and ('+', ('*', 3, 4), 5).

Dorian Turba
  • 3,260
  • 3
  • 23
  • 67