Factorial


#1



https://www.codecademy.com/courses/python-intermediate-en-rCQKw/0/5?curriculum_id=4f89dab3d788890003000096#


Not sure why I am getting this error. The math seems to work for me on paper. Need help understanding what I am missing. The error...
Oops, try again. factorial(2) returned 3 instead of 2


My paper math says that the following steps would execute with this code:
factorial(2)
total = 2
(range = 2, 1)
for 2 in range
if 2 is > than 1 (true so execute)
total = 2 * 2 - 1 (total = 2)
for 1 in range
if 1 is > than 1 (false so break)
return total which is 2

(I figured out what went wrong on this as I was typing this post. I needed the () around the i - 1 so that it calculated before being multiplied by total)

Figured I'd post it any ways to get feed back and hope that it helps someone else. Any suggestions on how to simplify this or make it cleaner? Not a fan of the way I did the range function, seems less than ideal. Any suggestions?


def factorial(x):
    total = x
    for i in range(x, 0, -1):
        if i > 1:
            total *= i - 1
    return total


#2

Simplified it a bit more by taking out the total variable and now have this:

def factorial(x):
    for i in range(x, 0, -1):
        if i > 1:
            x *= (i - 1)
    return x

#3

your flaw is here:

total = 2 * 2 - 1 (total = 2)

how is 2 * 2 -1 possibly 2? 2 * 2 is 4. 4 - 1 is 3. Can you explain how you got to two?


#4

Had the order of operations wrong. was thinking that - would execute before *. Adding () fixed this. Thanks for the feedback


#6

do you understand it all do your have any questions about it?


#7

Only question is if there is a better way to do range(x, 0, -1)


#8

range() is a good approach. The other (the following is a personal opinion, not a fact) solution is to declare a variable total and give it a value of 1

then use a while loop to get through all the values while decreasing x, its similair to your range solution.

the other solution (which again, opinion) i dislike is using recursion


#9

This topic was automatically closed 7 days after the last reply. New replies are no longer allowed.