Calculator that parses S-expressions

This is my first program I can call 'something'. I would be glad to hear what you think of it, so I wouldn't pick up bad habits further on.

It catches errors, but requires numbers to be separated from operators. It ignores a lot of characters, therefore (+ 2 2) is as valid as (+ 2 blargh 2)

100 lines of code + comments that should make everything clear:

```
# This program evaluates an arithmetic problem written as an S-expression.
# Thus 3 + (7 4 2) + 1 + (6 / 5) + 8 becomes (+ 3 (* 7 4 2) 1 (/ 6 5) 8).
# IMPORTANT NOTE: every number must be separated from an arithmetic operator by
# white space to avoid confusion between arithmetic operation and indicator of
# positiveness or negativeness of the number.(+3 7) is INVALID and evaluates to
# 7 because '(+3' bit is simplified to '+'. Instead use (+ 3 7) or (+ +3 +7)!

# The program loop.
def mainLoop():
while True:
expression = getExpression()
if (expression.upper() == "EXIT"):
break
expression = formatExpression(expression)
result = evaluate(expression, 0)[0]
print (result)

# Asks user to enter an expresion and returns the input.
def getExpression():
print("Enter \"EXIT\" to exit. Numbers must be separated from operators.")
expression = input("Enter expression: ")
return expression

# Formats an expression so it would be easy to be manipulated further on.
def formatExpression(expression):
exp = []
expression = leaveCertainChars(expression, ")0123456789.+-*/ eE")
expression = detachBrackets(expression)
expression = expression.split()
for item in expression:
if isNumber(item):
exp.append(item)
else:
for character in item:
if isOperator(character) or character == ')':
exp.append(character)
return exp

# Leaves only the characters that really matter.
def leaveCertainChars(string, chars):
new_string = ""
for character in string:

Following PEP-8

• High level descriptions of functions should be docstrings

• The name should be snake_case

def is_operator(string):
    """ Returns True if string is an operator otherwise returns False."""
    if string == '+' or string == '-' or string == '*' or string == '/':
        return True
    else:
        return False

Using a constant to simplify

Also the function above can be simplified and made more readable by defining a constant at the start of your file:

OPERATORS = "+*/:"

the function can then become:

# By the way, usually `string` is multiline so `token` may be better
 def is_operator(token): 
    """ Returns True if string is an operator otherwise returns False."""
    return token in operators

Be exact

def leave_certain_chars(string, chars):
    """Leaves only the characters that really matter."""
    new_string = ""
    for character in string:
        if character in chars:
            new_string += character
    return new_string

Code must be precise: that really matter tells nothing to me, certain gives a feeling of uncertainty that should not be present when reading code...

Also it is a very verbose way to accomplish such a simple task. My rewrite is as follows:

def also_in_second(main_string, second_string):
     """
     Given two strings, returns all the chars of the first
     string that are also present in the second one.

     >>> also_in_second("hello world", "hde")
     'hed'
     """
     return ''.join([i for i in main_string if i in second_string])

Go built-in whenever possible

# Converts each ')' to ' )'
def detachBrackets(string):
    new_string = ""
    for character in string:
        if character == ')':
            new_string += " )"
        else:
            new_string += character
    return new_string

What you want to do above is called replacing and you should use the standard library function:

def detach_brackets(expression):
    """Adds a space before closing brackets"""
    return expression.replace(')',' )')

Make the habit of doctesting

In math code, correctness of complex algorithms must be achieved, automatic tests help us a long way in doing that, for example:

import doctest # Start of your script

doctest.testmod() # End

setup done, the tests will run each time we execute the module, now:

def formatExpression(expression):
    """
    >>> formatExpression("(+ 9 (* 3 2))")
    ['+', '9', '*', '3', '2', ')', ')']
    >>> formatExpression("(+ 5 (- 3 (* 2 11)) 9)")
    ['+', '5', '-', '3', '*', '2', '11', ')', ')', '9', ')']
    """
    exp = []
    expression = leaveCertainChars(expression, ")0123456789.+-*/ eE")
    expression = detachBrackets(expression)
    expression = expression.split()
    for item in expression:
        if isNumber(item):
            exp.append(item)
        else:
            for character in item:
                if isOperator(character) or character == ')':
                    exp.append(character)
    return exp

It looks like this function makes a list and removes starting brackets and spaces, let me implement it again:

def formatExpression(expression):
    """
    >>> formatExpression("(+ 9 (* 3 2))")
    ['+', '9', '*', '3', '2', ')', ')']
    >>> formatExpression("(+ 5 (- 3 (* 2 11)) 9)")
    ['+', '5', '-', '3', '*', '2', '11', ')', ')', '9', ')']
    """
    expression = expression.replace('(','( ').replace(')',' )')
    return [i for i in expression.split() if i not in ('(',' ')]

Automated testing allows for simpler, faster and more secure simplification of existing code.

Some user interface notes

Avoid all caps

WRITING ALL CAPS is considered very uneducated and is almost the same as shouting, now imagine you asked a friend to calculate a wrong expression and he replied shouting "THIS IS NOT VALID!". You would fell offended right?

Reduce clutter

You repeat the same messages over and over again, the user can remember them in my opinion. Your user interface should look like:

Welcome to the S-expression calculator.
Enter your expression at the '>' prompt to get the result and
enter "Exit" to exit.

> (+ 2 9)
11

> 2 + 2
Error: Bracket missing.

>


StackExchange Code Review Q#86055, answer score: 8