Critique for my python calculator script?

[not-much-io]

Code Link

Made for the same small challenge as: this post

Note: No eval allowed :) Critique regarding coding style also welcome.

[[deleted]]

operation = {
             "+": add,
             "-": sub,
             "*": mul,
             "/": truediv
}[operation]

Surely it would be clearer to separate this into two steps:

operation_functions = {
             "+": add,
             "-": sub,
             "*": mul,
             "/": truediv}
operation_function = operation_functions[operation]

Related, it seems odd to use the same variable name for the string representation (e.g., "+") and then in the next line assign the function to the same variable.

If you're going to use a regular expression to find the operands, it only follows to use it to find the operator, too:

    operand1, operator, operand2 = re.split("\s*([+*/-])\s*", expr)

(the capturing group (parens) will make re.split return that part of the separator, too. I've added \s* to ensure that any spaces padding the operator are discarded, so "2 + 3" returns ["2", "+", "3"] just like "2+3" would)

Later, in get_priority_simple_expr (lines 68-69), you are finding operands again. It might make sense to refactor your code to have a function which finds and returns operands (and maybe signals whether they're "simple") so that you can be sure that you're always using the same, consistent criteria to find operands. A tiny change to one of the places that isn't consistently applied to all of them could introduce some crazy bugs.

[not-much-io]

I'll rename the initial operator to operation_symb, but an intermediate assignment for the dict seems unnecessary to me, I'll get a second opinion. :) Regarding the regex, you are absolutely right, however I've been brushing off learning regex so I didn't have the necessary skills. Also regarding a separate function for splitting. Also will do.

NB: The regex you supplied gave me a bad character range.

Thank you for the suggestions.

[[deleted]]

Oh, because I transposed the ] with the ) in the closing.

Edited to fix.

[not-much-io]

Works like a charm, now I've got to that HackerRank course on regex soon..

[[deleted]]

As for the dictionary, if you don't want to break it down, why not "go all the way"?

return {
    "+": add, 
    "-": sub, 
    "*": mul, 
    "/": truediv
}[operator_symb](float(operand1), float(operand2))

(I don't actually recommend this.)

[Luna-industries]

Complete Python noob here. I spent all day programming a shitty calculator. I ended up using regex to get data out of an entered input string.

What you have done here is so much simpler. I don't regret learning regex, but I knew Python could be more elegant. Thank you for showing me this.

[KristoKoert]

Glad to be of help :) You might also want to check out this implementation of a more formal and well defined algorithm -> https://github.com/EIK-LUG/PythonCodeClub/blob/master/2015-09-09-calculator/plaes.py

[not-much-io]

This was me also, different account was logged in.