HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Parsing s-expression structure into tree and summing the paths

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
expressionthesummingintopathsparsingstructureandtree

Problem

I'm new to Python, just attempted the task here.

The part I found hardest was parsing the expression into the tree structure. I was originally trying to build a regular tree structure (i.e. a Node object with left and right nodes), but without any logic for the insertion (i.e. newNode node then insert right) I couldn't find a way.

In the end I've used Python's lists to kind of replicate the expression structure, and walk the paths as they're created. Each time I find a leaf, I calculate the cumulative sum, pop the last added node, and carry on.

The one part of the code I really don't like is the way I'm finding leafs:

if tree and expression[i-1:i+3] == ['(',')','(',')']:


and I don't like that I've done:

pair[1].replace('(', ' ( ').replace(')', ' ) ').split()


twice.

Any guidance on any part of this - style or just general approach and logic would be great.

```
def pairs(value):
""" Yields pairs (target, expression) """
nest_level = 0
expr = ""
target = 0

value = value.replace('(', ' ( ').replace(')', ' ) ').split()
for x in value:
if x.isdigit() and not expr:
target = x
else:
expr += x

if x is '(':
nest_level += 1
elif x is ')':
nest_level -= 1
if nest_level is 0:
yield target, expr
expr = ''
target = 0

def main():
with open('input') as f:
expr_input = f.read()

level = 0
current_target = 0

for pair in pairs(expr_input):
current_target = pair[0]
# stack representing the 'current path'
tree = list()
# store the cumulative total of each path for this expression
cumulative_totals = list()
running_total = 0

expression = pair[1].replace('(', ' ( ').replace(')', ' ) ').split()
for i, s in enumerate(expression):
if s is '(':

Solution

-
Avoid indentation.

print "no"


When you need this amount of spaces in front of your code, I'd say no too.

This is clearly a sign that what you're writing could be extracted into a function call, our main could be a chain of function calls instead of a big clump of code.

with open('input') as f:
    parseFile(f)

def parseFile(...):
    ...
    for pair in pairs(expr_input): parsePair(pair)
    ...

def parsePair(...):
    ...
    for i, s in enumerate(expression): addExpressionToSummation(i, s)
    ...


Of course, this won't work out of the box; that's what object-oriented programming will help with.

Also, if you need to map one-to-one, use list comprehensions or functions like map.

-
Regular expressions might sometimes be valuable.

value = value.replace('(', ' ( ').replace(')', ' ) ').split()


can be

value = re.sub(r'(\(|\))', r' \1 ', value).split();


which can be improved to not add multiple spaces by restricting the match or by doing another replace where you replace r' +' by r' '. Of course this example might not be worth it, but if you've got to do heavier duty then you'll quickly want to resort to a regular expression instead of much longer trial-and-error code.

-
Conditions can be functions, too.

# "is leaf?" ugh.
if tree and expression[i-1:i+3] == ['(',')','(',')']:


Imagine that'd be

if isLeaf(tree, expression):


Oh look, our documentation line is gone. ;)

Code Snippets

with open('input') as f:
    parseFile(f)

def parseFile(...):
    ...
    for pair in pairs(expr_input): parsePair(pair)
    ...

def parsePair(...):
    ...
    for i, s in enumerate(expression): addExpressionToSummation(i, s)
    ...
value = value.replace('(', ' ( ').replace(')', ' ) ').split()
value = re.sub(r'(\(|\))', r' \1 ', value).split();
# "is leaf?" ugh.
if tree and expression[i-1:i+3] == ['(',')','(',')']:
if isLeaf(tree, expression):

Context

StackExchange Code Review Q#18342, answer score: 4

Revisions (0)

No revisions yet.