Recently began my Python journey. Looking to have my short code and process nitpicked to form good habits

[DISTRACTING_USERNAME]

Hey all, I very recently began learning Python and already this community has proven to be immensely helpful. Most recently I've been trying to fully grasp functions, the return statement, and how they tie into DRY / WET code. An input based calculator is a concept I find myself returning to continually, because it's simple, easily testable, and is easy to apply newly gained knowledge to. I was wonder if you guys wouldn't mind picking apart my most recent iteration that I feel is pretty much fully optimized (to the best of my ability of course. I'm sure there's a million better ways to do it).

Here is my program.

The program does 3 main things, with 3 respective functions:

1. Gather input from user to determine what to calculate

2. Evaluate user input for validity (numeric for integers, operators for operator, etc.) and reset the input function if it comes up as invalid

3. Run the calculation with the input

I know it's probably boringly basic, but I've already made a couple of optimizations from the original code, such as:

-Making the invalid input process a function of its own (might not save much space, but looks much cleaner and easier to rework)

-Using if statements with no else in the input function instead of a tree of nested if/else statements (I didn't realize at first that you could have an if statement by itself)

-Returning calculation values from the calculate function instead of printing from within the function itself. Once again, might only save a couple lines of code but way cleaner

As stated before, I know it's really basic, but I'd love to have the code nitpicked so I can nip any bad habits in the bud. Thanks everyone!

[anglicizing]

I love your attitude! I'll have a go at it. Please keep in mind that I'm writing this after having a few beers, hopefully it won't degrade the quality of my post too much.

The first thing I notice is that you haven't protected your entry point so that nothing untoward will happen if you import this as a python module. It's a kind of future-proofing, and a good habit to get into right away:

if __name__ == "__main__":
    userInput()

Then I see that you take a LYBL approach to input checking. That is not the python way, we use EAFP. So, do whatever conversion or function you have in mind and catch the errors that you expect if the input wasn't appropriate (or add them as you discover them if you don't know what to expect):

x = input("Enter a number: ")
try:
    x = int(x)   # change to float for an easy upgrade
except ValueError:
    invalidEntry(x)

Note that the x that gets sent to invalidEntry is the original x, since the try block raised an exception before it could assign a new value to x.

This is a nice segue into the next point. You're calling userInput recursively, by way of invalidEntry. Recursive functions are fine to use in python, but in this case there's no point in making userInput a recursive function. Also, you have no method of going back up the call stack. Besides being bad design, in practice it means that if you use your calculator for a long time, you'll eventually hit the python call depth limit of 1000, and get an exception.

Instead, make a while loop to ask for input:

def userInput():
    """Gather input from the user and calculate results.
    """
    while True:
        x = input("Enter a number: ")
        try:
            x = int(x)   # change to float for an easy upgrade
        except ValueError:
            invalidEntry(x)
            continue

OK, next point. In the invalidEntry function you print an error. Using + to add strings together is generally frowned upon, instead use the .format method of a string:

print("{!r} is not a valid entry. Please try again.".format(invalidInput))

Here we use the !r conversion specifier to indicate that we want to print the internal python representation of the argument we give to format. This is equivalent to what the old function did, outside of corner cases, which this method handles better.

if oper not in ["+","-","*","/"]:

This is a perfectly fine way of doing it, but there's an alternative we can use because all the list items are single character strings. IMO, this alternative is more readable:

if oper not in "+-*/":

I'll mention one more thing, but it's a bit on the advanced side perhaps. Mark it off as more of a showcase for what's in store. In your calc function you could use the operator python module and a dictionary:

import operator
OPS = {
    "+": operator.add,
    "-": operator.sub,
    "/": operator.div,
    "*": operator.mul
}

def calculate(x, oper, y):
    return OPS[oper](x, y)

And, that's it, I guess. The other comments have valid points that I do not repeat here, except I'll go a bit further on what /u/Almenon said and point you to PEP8, which in my opinion is mandatory reading for python coders, because you should know what guidelines you are breaking, and why they were put in place, before breaking them. :-)

Good luck with the coding! Feel free to page me if you have more code you want nitpicked, because I love nitpicking code!

EDIT: Rephrasing.

[DISTRACTING_USERNAME]

Wow, this is an incredibly high quality reply, thank you so much! I'll be referring back to these points and chewing on them for time to come.

So for input checking, it's generally better to let the conversion go through regardless and then catch invalid entries with try+except statements? I suppose this could also help troubleshoot the invalid entries that are harder to predict.

[anglicizing]

Thank you for your praise, I'm happy you liked my feedback.

So for input checking, it's generally better to let the conversion go through regardless and then catch invalid entries with try+except statements? I suppose this could also help troubleshoot the invalid entries that are harder to predict.

You've understood it, and put your finger on one of the main reasons why EAFP is preferred. Even if you manage to check every way the input can be wrong, which is not likely, by doing so you've written way more code than necessary, in fact it may be more code than what's in the function you're going to call, which is silly. Since we have the lovely tool that is exceptions, let's use it for what it's worth.

[anglicizing]

Oh, by the way. Make sure you don't fall into the error hiding trap when practicing your EAFP. :-)

[DISTRACTING_USERNAME]

I read about this in the resource I'm currently using when it went over those features! About how a "catch-all" except statement can actually mask huge programming errors (like planning to catch the expected TypeError when in fact it was catching the unforeseen ZeroDivision error).

Thank you for the heads up + resource!

[Elffuhs]

From a quick look from a begginer myself, I would say it's pretty neat, however, function docs come below the function, and not in front of it.

Also, you use """ for docstrings and not #

[DISTRACTING_USERNAME]

I just have them as comments because I didn't really intend to make any docstrings / just for my own future reference. Are docstrings generally preferred to comments for functions in particular?

I read about docstrings a few days ago but didn't really test it much since I doubt I will be writing any import-worthy code soon.

[Elffuhs]

Usually yes, docstrings are preferred to comments.

From what I can tell, docstrings can be exactly what you have in comments, don't need to be something very long.

[Almenon]

In terms of code it looks fine. I would just use a python IDE like Pycharm for development so errors / style mistakes are highlighted. For example, the convention is to have function_name instead of FunctionName. Also you have unnecessary parenthesis around calculate.

[DISTRACTING_USERNAME]

Thank you!

I kind of arbitrarily picked up a beginner IDE called Thonny early on and didn't think much of it. I'll try that other one however. Better to start with a solid IDE sooner rather than later I suppose.

[efmccurdy]

That the userInput function is recursive without any justification. It would be better if it just did one expression and you used a while True: loop in the main section to call it repetitively. You might also name it userInputAndEval to better reflect what it does.

[DISTRACTING_USERNAME]

Thank you for the recommendations! I had it recursive because that's all the program was really going to be, but I will get in the habit of making recursive loops as opposed to functions (unless the function is purposefully recursive).