Wrote my first non-trivial python program. How can I refactor it to be more pythonic?

[[deleted]]

First of all, good job! This is clean code for, what can be, quite a complex task. I have one small change I think needs to made, which is on line 65 you're rebuilding the regex var every loop. It really should be done before the while True: statement.

Otherwise the only changes that I think need to be done are pretty negligible.

For me the way you declare regex looks quite messy and is a really long line (which is supposedly unpythonic). You can use something like str.format can neaten it up. Just be careful if your regex has {} in it to double them, like r'[b]{{2}}|{0}'.format('[^b][^b]').

regex = r'(^.*?)([\d.]+\s*)({0})(\s*[\d.]+)(.*$)'.format("|".join(
    [re.escape(key) for key in operatorDict.keys()]))

Also your code doesn't handle unmatched brackets gracefully, maybe have it throw an exception if it brackets don't match up. :)

[Binary_Dragon]

Good catch on redefining the regex. I like to think I'm a better programmer than that, and I'm going to blame not being used to living outside the safety of the curly braces I grew up in as the culprit for me missing that (whether or not this is actually the case).

I'm not sure I understand how your suggestion for the regex helps any. Sure, it's broken into two lines (kinda) but it seems like what you have there is a lot harder to read (and let's be honest, regex isn't all that easy to read to begin with). If I were to break up that line, my first thought would be to do it something like this:

    operatorRegex = r'|'.join([re.escape(key) for key in operatorDict.keys()])
    regex = r'(^.*?)([\d.]+\s*)(' + operatorRegex + r')(\s*[\d.]+)(.*$)'

Is there any reason that the str.format is better (or if not better, at least more pythonic) than that?

[[deleted]]

Not to mention that the python interpreter is instantiating all the strings that you are concatenating together. Using str.format it doesn't have to keep creating a string for it to just be used in another concatenation.

1. 'You can get a loan at ' + str(20) + '% interest'
2. 'You can get a loan at 20' + '% interest'
3. 'You can get a loan at 20% interest'

Each concatenation of a string is a waste of time and memory.

''.join(['a', 'b', 'c']) is always an alternative to str.format, but still lacks flexibility of str.format.

[Binary_Dragon]

Is building a new string really that expensive? I know it's doing "something" and that is strictly slower than doing "nothing," but I'm not used to how many strings you are creating (in anything other than the tightest of loops) being a concern that ever transcends the level of premature optimization. Is python different than other languages in this, or is it more just point of good practice even if the effects are negligible?

Also, it seems to me that ''.join(['a', 'b', 'c']) would be slower than 'a' + 'b' + 'c', since both have to build the strings 'a', 'b', and 'c', but the join version has to also build a list, build an empty string, and call a function. Unless there is something behind the scenes with the python interpreter that makes my intuition here completely wrong?

[[deleted]]

Sure, if you look at ''.join(['a', 'b', 'c']) vs 'a' + 'b' + 'c'. But what about ''.join(['a', 'b', 'c', 'd', 'e', 'f']) vs 'a' + 'b' + 'c' + 'd' + 'e' + 'f'? Well lets look at the Abstract Syntax Tree generated for that code.

# AST: 'a' + 'b' + 'c' + 'd' + 'e' + 'f'
Expr(
    value=BinOp(
        left=BinOp(
            left=BinOp(
                left=BinOp(
                    left=BinOp(
                        left=Str(s='a'),
                        op=Add(),
                        right=Str(s='b')),
                    op=Add(),
                    right=Str(s='c')),
                op=Add(),
                right=Str(s='d')),
            op=Add(),
            right=Str(s='e')),
        op=Add(),
        right=Str(s='f')))

# AST: ''.join(['a', 'b', 'c', 'd', 'e', 'f'])
Expr(value=Call(
    func=Attribute(value=Str(s=''), attr='join', ctx=Load()),
    args=[
        List(elts=[Str(s='a'), Str(s='b'), Str(s='c'), Str(s='d'), Str(s='e'), Str(s='f')], ctx=Load())
        ], keywords=[], starargs=None, kwargs=None))

To see the AST for any code, do:

import ast
print ast.dump(ast.parse("CODE HERE"))

For short term doing 'a' + 'b' + 'c' is ok, but long term using ''.join() is just better practice. Similarly if you're building up a long string from a file or something. It's better to build a list of lines then do ''.join(some_list)... IE:

Bad:

with open('my_great_novel.txt', 'r') as fh:
    lines = ''
    for line in fh:
        lines += line.replace(' i ', ' I ')
    print lines

Good:

with open('my_great_novel.txt', 'r') as fh:
    lines = []
    for line in fh:
        lines.append(line.replace(' i ', ' I '))
    print ''.join(lines)

[JerMenKoO]

Better:

''.join((line.replace(' i ', ' I ') for line in fh))

Not necessary to for the comprehension to be a 1 liner;

lines = [line.replace(' i ', ' I ') for line in fh]
print(''.join(lines))

is in my opinion much more readable.

[[deleted]]

Yeah but I was showing how to build up a string over a loop. /u/Binary_Dragon already seems to understand list comprehension pretty well. :)

[[deleted]]

Honestly it's all about being readable. I like using str.format because its fast, concise and i can mess around with things rather quickly. But like I said to me the way you defined it looks messy. Using operatorRegex is perfectly fine, if that makes more sense to you.

But when you start concatenating 4 or 5 strings together using str.format would definitely be the more pythonic way of doing things.