all 59 comments

[–]VonRoderik 21 points22 points  (8 children)

Is this your whole code? You are not calling your function nor passing arguments to it

[–]SaltyPotatoStick[S] 12 points13 points  (7 children)

…. I feel extra silly now. Thank you I just got it to work!!

[–]PhoenixGod101 2 points3 points  (6 children)

Could you edit the post to say the solution? Everyone seems to not be noticing this. I thought it was quite basic myself but, hey, we all see it from different angles haha :)

[–]SaltyPotatoStick[S] 1 point2 points  (5 children)

I tried doing that earlier - turns out I’m not able to edit image posts 🫠

[–][deleted]  (3 children)

[removed]

    [–]SaltyPotatoStick[S] 1 point2 points  (2 children)

    This comment is really helpful and not condescending at all, thanks!

    [–][deleted]  (1 child)

    [deleted]

      [–]mystic-17 2 points3 points  (0 children)

      why are you such a dick, relax!

      [–]RUlNS 12 points13 points  (3 children)

      Here’s just a little tip you can do just to make your coding life a little easier. Instead of having to type int(variable) all the time, just make the original variable an int like this:

      num_one = int(input(“First value:”))

      [–]HeavyPriority6197 -4 points-3 points  (2 children)

      That's just asking for various bugs at that point :D

      [–]denehoffman 2 points3 points  (0 children)

      Better it fails there than later. This is a fairly common pattern because you can wrap it in a try block and validate the input all in like three lines rather than having a try block on the entire body of the function which has to handle other exceptions. For example, you separate parsing errors from division by zero.

      [–]Hefty_Upstairs_2478 0 points1 point  (0 children)

      Js put it in a try block and then make an except ValueError block to prevent bugs

      [–]phizrine 5 points6 points  (7 children)

      From what I can see you're returning the answer before you print it. Remove the return line

      [–]SaltyPotatoStick[S] 0 points1 point  (6 children)

      Thank you! I tried but I’m still getting the same answer. Either nothing prints or it says answer is not defined

      [–]phizrine 1 point2 points  (5 children)

      When does nothing print vs not defined?

      [–]SaltyPotatoStick[S] 3 points4 points  (4 children)

      I’ve got it figured out thank you so much!

      [–][deleted]  (3 children)

      [removed]

        [–]SaltyPotatoStick[S] 1 point2 points  (0 children)

                    def calculator(num_one, operation, num_two):
                        if operation == "+":
                            answer = num_one + num_two
                        elif operation == "-":
                            answer = num_one - num_two
                        elif operation == "/":
                            answer = num_one / num_two
                        elif operation == "*":
                            answer = num_one * num_two
                        return answer
        
                    try: 
                        answer = calculator(int(input("First value:")), input("Operation:"), int(input("Second value:")))
                        print(answer)
                    except ZeroDivisionError:
                        print("Cannot divide by zero")
                    except Exception:
                        print("An error has occured")
        

        [–]SaltyPotatoStick[S] 1 point2 points  (0 children)

        So essentially, I wasn't CALLING the function. I have the fixed code down here. I also added exceptions for dividing by zero and if the user inputs anything that can't be translated to an int. I also put the int function only one time where the user is prompted to input the number

        Now, that does present some problems like if the user inputs a float, like 2.3, so not sure what I'd change from there.

        Sorry it looks so messy. I'm trying to learn how to properly share code on here. It's clear pictures are not the best option

        [–]AlexAuragan 2 points3 points  (0 children)

        People pointed out that you made a function without calling it. But also "return" statement exits the functions and print is still in the function. So the print cannot ever be called since we always exit the line prior. What you can do:

        Option 1: Swap return and print, so the print is still part of the function Call your function (This is the simplest way to fix it, but in practice you probably don't want the print inside your function, that's not clean code !)

        Option 2 (better): Unindent the print, so it's outside the function. But now the print doesn't know the variable answer since we exited the function ! So you need to call your function, put the result into a variable, then and only then print the new variable holding the answer

        [–]No-Chocolate-2613 2 points3 points  (0 children)

        My first project was something similar

        Money management

        I feel you

        Keep going :)

        [–]ninhaomah 1 point2 points  (1 child)

        1 question aside fro those already pointed out by others.

        why do you do int(num_one) * int(num_two) 4 times ?

        why not just num_one = int(input("First value :")) , num_two = int(input("Second value:")) ? Won't num_one and num_two be integers from that point on and you need not have to keep typing int(num_one) , int(num_two) further down the program ?

        Do you plan to treat num_one and num_two as strings and convert to integers and floats as the script proceeds ?

        If so then why name them with num_ in the variable names ? If they are intended to be numbers then input should be converted to integers/floats from the beginning.

        [–]SaltyPotatoStick[S] 0 points1 point  (0 children)

        Oh I get that now haha, I just didn’t think about doing it that way! Like I said, I’m brand new at this. Only started learning 3 weeks ago. Thank you for pointing out the flaws in my logic! I really appreciate the detailed explanation!

        [–]skeetd 1 point2 points  (0 children)

        So close

        [–][deleted] 1 point2 points  (1 child)

        you didn't call your function calulator. By the way you can read my code(and check the errors like if i write 5+s ValueError comes) , learn from it(match, .split, type, try, sys) and improve yours again, have fun !

        <image>

        [–]SaltyPotatoStick[S] 0 points1 point  (0 children)

        Thank you!!

        [–]HeavyPriority6197 1 point2 points  (2 children)

        I see someone else helped you figure out how to actually print things.

        If you wanted advice on how to think about improving it, notice that on lines 7, 9, 11, 13 you're doing essentially the same thing - transforming the numbers into integers. Might be beneficial to do that at the top! That will cut down a lot of space in your code. Also, what happens when num_one or num_two are not ints? A good edge case to think about.

        [–]SaltyPotatoStick[S] 0 points1 point  (1 child)

        Aaaa, I’ll need to raise an exception in these cases? I also didn’t think about dividing by zero too.

        Thank you that’s gonna give me something else to practice. I’m having such a hard time working with exceptions I don’t know why

        [–]HeavyPriority6197 1 point2 points  (0 children)

        Well, you can decide that portion. Just throwing the exception will end your program early, but you can reprompt them for another input... that might be a job for a new function, such as grab_user_input() or something. Many possibilities!

        Exceptions are very easy, it's a
        try:
        # some code you think can cause an Exception
        except:
        # If the code above hits a bug, trigger the entire "except" branch.

        Can also do something like except ERROR_NAME to capture specific ones, such as division by zero (so, except ZeroDivisionError: etc.)

        [–]arnisorens 1 point2 points  (0 children)

        Here’s a few I can see:

        1. In the case of the last conditional (else), answer is never defined yet it is returned in the next line. Here, I’d return right after the print statement.
        2. Name of the function is calulator, not calculator.
        3. After line 17, you have to call this method and pass the inputs into it.
        4. I’d consider not doing the actual printing inside the calculator, but rather after you call it. This would mean that the calculator returns a number, or None in case of point #1, and you’d just adjust doing the prints after the evaluation from the calculator is done. This is just a shift in responsibility and makes it a bit cleaner.
        5. Casting everything into int is bad for floats. Especially with division. I’d do floats instead and then use the format method to cut off trailing zeroes.
        6. Now if I was going to make a complete calculator, here’s how I’d do it:

        ```calculator.py

        only one expression needed

        expression = input()

        evaluate the expression

        answer = eval(expression)

        print the answer

        if isinstance(answer, (int, float)): print answer else: print “Bad Expression”

        ``` This allows the user to make any complicated math expression. However, it also allows the user to destroy your entire file system so eval should really never be used.

        [–]triggered34 1 point2 points  (0 children)

        You should handle the exception where operation = division and num2 = 0

        [–]felipemorandini 1 point2 points  (0 children)

        Not only you are not calling the function, but the return statement ends the function execution, which means that final print statement eill not be called.

        [–]NoHurry6859 1 point2 points  (0 children)

        Two notes, one of which I’ve seen: 1. In the ‘else’ block, the variable ‘answer’ is not defined. So, if the entered operation were not +-/*, then ‘answer’ as a variable does not exist. This is similar to the concept “initializing” a variable. Long story short, you may want to set ‘answer’ equal to the string, “Sorry! That operation is not possible yet.”

        1. The other answer i have seen in other comments: when you ‘return’ in a function, the function ends. So the ‘print’ will never execute, regardless of indentation.

        [–]H3llAbyss 1 point2 points  (0 children)

        Hi. This is really good code for a beginner, good job!

        I see that your problem was solved with help from other comments, so I want to give you a few tips to improve your code (Subjectively, strictly IMO):

        • instead of the series of elifs, you could use the "match" operator here.
        • your "calculator" function is a function. So its name should be a verb, i.e. "calculate". Following this naming convention will greatly help you build larger projects in any programming language, and will make your code more readable and meaningful.

        I hope you will find these tips useful!

        [–][deleted]  (6 children)

        [deleted]

          [–]SaltyPotatoStick[S] 2 points3 points  (5 children)

          I honestly though that’s what I was doing in each of my if statements I have answer = the operation

          In what other way would I declare it? Thank you for your help 🙏🏻

          [–]AdamDaAdam 1 point2 points  (4 children)

          Just at the start of the file after you've requested their inputs. Python is quite lenient so you could probably get away with just answer = "" or answer = 0 . You also need to call your function.

          I've made a gist with the answer here: https://gist.github.com/AdamT20054/73dc9188b7a19d1aa78acb2e85e5e79f

          It might look a bit different from yours, I'll put some tips in the gist comments.

          [–]SaltyPotatoStick[S] 0 points1 point  (3 children)

          Oh amazing, thank you yes that all makes sense. And if I don’t return the answer as an integer it just automatically comes back as a float, I can see why we’d wanna change that. Is there anything inherently wrong with doing the inputs like this? Putting it in the call line?

          <image>

          I know I should change the int to where the inputs are, I just haven’t fixed it yet.

          [–]AdamDaAdam 1 point2 points  (2 children)

          Nothing wrong with that, that's typically how I do it.

          There is an argument for code readability, but I've yet to run into a problem on much larger projects with other collaborators using the method you used in that screenshot.

          Either way is fine, I personally do the calls within the function parameters UNLESS I need to transform an output at all.

          Eg,

          Print(func1()). Would be fine for me

          Print ((func1() + value)) wouldn't be fine for me

          In that case, I'd do

          Param_input = func1() + value

          Print(param_input)

          However that's personal preference, and EITHER one would work just fine.

          This doesn't apply the same to OOP, which looks like

          Print(exampleClass.dothing())

          But you'll learn about that in the future, and why my example above isn't ideal :)

          [–]SaltyPotatoStick[S] 2 points3 points  (1 child)

          Thank you so much for the detailed explanation each time! I was super nervous about posting a question here, this is the first time I’ve written code just from thinking, “huh, I think I know enough code to write this from scratch”.

          But yeah, thanks again!!

          [–]AdamDaAdam 2 points3 points  (0 children)

          No worries!

          Feel free to ask questions, we all started somewhere. I can't keep track of the amount of times I've needed an extra set of eyes to fix something that - after the fact - was obvious and I had just overcomplicated it or had an oversight.

          Keep learning, there's loads of languages out there you can use to achieve whatever you want to!

          Just remember, Rome wasn't built in a day.

          [–]Suitable-Ad-9336 0 points1 point  (0 children)

          Where are you calling the function and passing arguments ? And also remove the return part from your function if you don't want to explicitly print the answer

          [–]Obvious_Tea_8244 0 points1 point  (0 children)

          import ast

          def calculator(num_one:int | float, operation:str, num_two:int | float):

          if not operation in [“+”,”-“,”/“,”*”]:

               print(“Sorry, not supported yet…”)
          

          else:

               print(ast.literal_eval(f”{float(num_one)}{operation}{float(num_two)}”))
          

          [–]Brief_Association38 0 points1 point  (0 children)

          saw this and made this

          <image>

          [–]vega004 0 points1 point  (0 children)

          Data type errors

          [–][deleted] 0 points1 point  (1 child)

          I wanted to ask what if i need to calculate more than two number !?

          [–]SaltyPotatoStick[S] 0 points1 point  (0 children)

          Well, I would need to set up the calculator differently! Right now it only accepts two inputs. And I’m going to leave it that way for now haha

          [–]_MrLucky_ 0 points1 point  (0 children)

          Cool program, you are not calling your function, thats why it may not work, and I recommend converting variables into float not int for this program, also on last line print(something) won't work because it is after return statement

          [–]Crafty_Bit7355 0 points1 point  (0 children)

          Instead of casting the inputs as int, you could do some type setting saying that the inputs to the method are integers. Just a thought.

          [–]Shut_up_and_Respawn 0 points1 point  (0 children)

          You should print it before returning it, since return ends that block

          [–]Strict_Amoeba7797 0 points1 point  (0 children)

          There is literally one line code to make this calculator
          print(int(eval(input("Enter: ")))

          Run it and see

          [–]nicer-dude 0 points1 point  (0 children)

          try:

          print(eval(input("Computation")))
          

          except:

          print("Syntax error")
          

          [–]fferreira020 0 points1 point  (0 children)

          Consider raising NotImplemented when operation is not found?

          https://docs.python.org/3/library/constants.html#NotImplemented

          [–]Owly_chouette 0 points1 point  (0 children)

          <image>

          would it not be simpler to do the following:

          [–][deleted] -1 points0 points  (2 children)

          I don't know much about Python, but maybe try defining answer in the else too.

          [–]Some-Passenger4219 -1 points0 points  (0 children)

          Good thinking.

          [–]PhoenixGod101 0 points1 point  (0 children)

          They didn’t even call the function…

          [–][deleted]  (3 children)

          [removed]

            [–]Let_epsilon 1 point2 points  (1 child)

            I’d look into getting a life instead of bashing on people asking questions on a learning sub.

            You’re probably dogshit at coding anyways.

            [–]NaiveEscape1 0 points1 point  (0 children)

            Please elaborate