2
\$\begingroup\$

Would appreciate a review on this. The program does work as it is intended to. However I am unsure if there is a more elegant or better way to execute this process with less code.

def main():

    print ('This program takes two numbers and multiplies them.') 
    print ('the result is multiplied by a third number') 
    print ('entered by the user too') 

    procedure()

def procedure():

    n = float(input('Enter a number: '))
    n2 = float (input('Enter a number: '))        
    value = n * n2
    y = value
    print value

    n3 = float(input('Multiply by: '))

    while n3 != '0':

        op = n3 * y  
        y = op        
        print y

        n3 = float(input('Multiply by: '))

main()
\$\endgroup\$
6
\$\begingroup\$

Your program makes inconsistent use of Python 2 and 3. For 3.x you need print('Hello World!'), for 2.x you need print 'Hello World!'. I'm guessing that you are using 2x because you say your program works and print value (as above) only works in 2.x. Note that in 2.x, input(prompt) is equivalent to eval(raw_input(prompt)). In 3.x, input() now means raw_input() and raw_input() has been removed. Given 2.x, your code already eval()s input, no you don't need float(). However, float(raw_input()) is safer so I'll use that below. Also, the use of n3 != '0' never works because n3 is a float not a string. Finally, I've cleaned up the variable names and removed unnecessary assignments.

number1 = float(raw_input('Enter a number: '))
number2 = float(raw_input('Enter a number: '))
result = number1 * number2

print(result)

number3 = float(raw_input('Multiply by: '))

while not number3 == 0:
    result2 = number3 * result
    print result2

    number3 = float(raw_input('Multiply by: '))
\$\endgroup\$
4
\$\begingroup\$

Infinite loop

Your program is an infinite loop, since a float will never equal a string.

Since you claim that this program works as intended, I'll take you at your word, and say that the loop should be written using while True: ….

Variables and functions

There are just too many variables being used for no good reason. In particular y and value are redundant.

The variable names aren't meaningful, and neither is your function name.

It is customary to introduce your main code with the boilerplate if __name__ == '__main__': ….

Input / output

You do float(input(…)) multiple times. It would be worthwhile to extract that into a function.

To print a multi-line string, write a """multiline string""". In this case, the string can do double duty as your program's docstring, so that's how I would write it.

Suggested implementation

"""This program takes two numbers and multiplies them.
the result is multiplied by a third number
entered by the user too"""

def prompt_float(prompt):
    return float(input(prompt))

def repeated_multiply():
    product = prompt_float('Enter a number: ') * \
              prompt_float('Enter a number: ')
    while True:
        print(product)
        product *= prompt_float('Multiply by: ')

if __name__ == '__main__':
    print(__doc__)
    repeated_multiply()
\$\endgroup\$
  • \$\begingroup\$ See the bit in my answer about Python 2/3. The OP is using Python 2, hence input() already evals. In 2.x, float(input(prompt)) is equivalent to float(eval(input(prompt))) in 3.x. \$\endgroup\$ – Bardi Harborow Jan 11 '15 at 10:35
  • \$\begingroup\$ @BardiHarborow I saw your preexisting answer, and decided not to make an issue of the Python 2/3 distinction so that I could focus on other issues. Instead, I just suggested a solution that could work in both versions. \$\endgroup\$ – 200_success Jan 11 '15 at 10:38
  • \$\begingroup\$ That's fine, I'm just being pedantic. =) \$\endgroup\$ – Bardi Harborow Jan 11 '15 at 10:41
1
\$\begingroup\$

Variable name:

  • Have a meaningful name for the variables and not n, n3, etc. Instead product , multiplier, etc would be good.

Others:

  • using single print instead of multiple. A linebreak('\n') would be useful.
  • What if y = 0 from n * n2, then its a futile multiplication loop.
  • Multiple variables for the same purpose: value, y, op.
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.