¿Qué está mal con mi código factorial en Python?

Tengo el siguiente código para calcular el factorial de un número en python. pero no pude entender por qué recibo la respuesta ya que 1. ¿alguien puede corregir mi código? Quiero calcular el factorial sin usar recursión.

def factorial (n): result =1 num = n while n<1: result = result * num num = num -1 return result factorial(5) 1 

 while n < 1: 

en cambio debería ser

 while num > 1: 

Otros señalaron lo que está mal con su código, pero quería señalar que una función factorial realmente se presta a una solución más funcional (como en: progtwigción funcional); esto evita el problema de obtener la condición para el bucle while por completo porque no tiene ningún bucle. La idea es que el factorial de n es el producto de 1..n , y el producto se puede desactivar muy fácilmente con la función de reduce de Python. Para evitar perder el rendimiento fuera de la vista, esto es lo que mi intérprete de Python 2.7 le da a su código (fijo):

 python -m timeit -s "import original" "original.factorial(10)" 1000000 loops, best of 3: 1.22 usec per loop 

Una versión más corta (de una sola línea) que es más declarativa es posible:

 def factorial(n): return reduce(lambda x,y: x*y, range(1, n+1)) 

… ay, es más lento:

 python -m timeit -s "import func1" "func1.factorial(10)" 1000000 loops, best of 3: 1.98 usec per loop 

Sin embargo, esto se puede resolver utilizando xrange lugar de range y operator.mul lugar de un lambda personalizado:

 import operator def factorial(n): return reduce(operator.mul, xrange(1, n+1)) 

Y para mí, esto es incluso más rápido que el código original:

 python -m timeit -s "import func2" "func2.factorial(10)" 1000000 loops, best of 3: 1.14 usec per loop 

Personalmente, eliminaría la reduce llamadas para hacer que el código sea aún más claro (a expensas de un poco de rendimiento):

 import operator def product(it): return reduce(operator.mul, it) def factorial(n): return product(xrange(1, n+1)) 

Me gusta esta versión por ser rápida y por ser explícita: el factorial se define como el producto del rango [1..n + 1 [(es decir, se excluye n + 1). La diferencia de rendimiento se hace más evidente si intenta calcular el factorial de números más grandes:

 python -m timeit -s "import original" "original.factorial(30)" 100000 loops, best of 3: 5.25 usec per loop 

contra

 python -m timeit -s "import func3" "func3.factorial(30)" 100000 loops, best of 3: 3.96 usec per loop 

while 5 < 1 siempre es falso, entonces se devuelve el result = 1 . Eso está mal.

Veamos:

  1. Establece n=5 cuando llama a la función.
  2. Le dices a Python que mientras n < 1 hace cosas.
  3. n ya es más grande que 1 , no ejecutará el código while .
  4. Su código devuelve el result , establecido en 1 en la primera línea de la definición.

Como explicó Simon Gibbons, su código tiene

while n < 1:

en lugar de

while num > 1:

Por lo tanto, tiene menos que en lugar de mayor que , por lo tanto, la prueba en su statement while fallará inmediatamente. Sin embargo, si lo cambiaba a while n > 1: sería un bucle para siempre, ya que nunca cambia el valor de n dentro del bucle while.

Haresh Shyara publicó una versión corregida de su código, reproducida aquí:

 def factorial(n): result = 1 while n > 1: result = result * n n = n - 1 return result 

Tenga en cuenta que este código no se molesta en copiar n a num , solo usa n directamente. Esto no afectará el argumento con el que llama a la función porque

  1. Los enteros de Python son inmutables y
  2. n = n - 1 realidad crea un nuevo objeto local llamado n .

Me inspiró la respuesta de Frerich Raabe para escribir un progtwig para hacer los tiempos de las diversas soluciones que se ofrecen aquí de una manera más sistemática. También math.factorial() y una función simple for bucle que acabo de juntar.

He optimizado las funciones que llaman a operator.mul poco definiendo mul = operator.mul , pero tuve que proporcionar un parámetro initial de 1 a las funciones que usan reduce() para que no fallen en factorial(0) (que debe devolver 1).

Aproximadamente he ordenado las funciones de la más rápida a la más lenta.

Acabo de mejorar este progtwig para que sea un poco más fácil ejecutar varias pruebas y agregar nuevas funciones para probar. Además, ahora imprime una breve descripción de cada función, utilizando la cadena de documentación de la función. Y antes de ejecutar las pruebas de tiempo, verifica que cada función calcula los valores correctos.

 #!/usr/bin/env python ''' Test and time various implementations of the factorial function From https://stackoverflow.com/q/28475637/4014959 Written by PM 2Ring 2015.02.13 ''' import operator import math from timeit import Timer factorial0 = math.factorial def factorial0a(n): ''' call math.factorial ''' return math.factorial(n) def factorial1(n): ''' for loop''' p = 1 for i in xrange(2, n+1): p *= i return p mul = operator.mul def product(it): return reduce(mul, it, 1) def factorial2(n): ''' reduce with op.mul ''' return reduce(mul, xrange(1, n+1), 1) def factorial3(n): ''' call product() ''' return product(xrange(1, n+1)) def factorial4(n): ''' while loop ''' result = 1 while n > 1: result = result * n n = n - 1 return result def factorial4a(n): ''' while loop with assignment operators ''' result = 1 while n > 1: result *= n n -= 1 return result def factorial5(n): ''' recursive ''' if n <= 1: return 1; else: return n*factorial5(n-1) def factorial6(n): ''' reduce with lambda ''' return reduce(lambda res, val: res*val, xrange(n, 0, -1), 1) funcs = ( factorial0, factorial0a, factorial1, factorial2, factorial3, factorial4, factorial4a, factorial5, factorial6, ) def verify(n): ''' Check that each function calculates the same result as math.factorial ''' r = xrange(n) fac = [factorial0(i) for i in r] rc = True for func in funcs[1:]: for i in r: v = func(i) if v != fac[i]: print 'Error: %s(%d) returns %d instead of %d' % (func.func_name, i, v, fac[i]) rc = False return rc def time_test(arg=10, loops=100000, reps=3): ''' Print timing stats for all the factorial functions ''' print 'Arg = %d, Loops = %d, Repetitions = %d' % (arg, loops, reps) for func in funcs: #Get function name and docstring try: fname = func.func_name fdoc = func.__doc__ except AttributeError: #Math.factorial has no name, and we can't modify its docstring fname = 'factorial0' fdoc = ' math.factorial itself ' print '\n%s:%s' % (fname, fdoc) t = Timer('%s(%d)' % (fname, arg), 'from __main__ import %s' % fname) r = t.repeat(reps, loops) r.sort() print r print '\n' def main(): if not verify(100): exit(1) time_test(arg=5, loops=500000, reps=4) time_test(arg=10, loops=200000, reps=4) time_test(arg=50, loops=100000, reps=4) if __name__ == '__main__': main() 

salida

 Arg = 5, Loops = 500000, Repetitions = 4 factorial0: math.factorial itself [0.30838108062744141, 0.3119349479675293, 0.31210899353027344, 0.32166290283203125] factorial0a: call math.factorial [0.62141299247741699, 0.62747406959533691, 0.63309717178344727, 0.66500306129455566] factorial1: for loop [1.4656128883361816, 1.476855993270874, 1.4897668361663818, 1.5052030086517334] factorial2: reduce with op.mul [1.5841941833496094, 1.5868480205535889, 1.6007061004638672, 1.6253509521484375] factorial3: call product() [1.8745129108428955, 1.8750350475311279, 1.8822829723358154, 1.9097139835357666] factorial4: while loop [1.1264691352844238, 1.1348199844360352, 1.1348659992218018, 1.178135871887207] factorial4a: while loop with assignment operators [1.1867551803588867, 1.1881229877471924, 1.1893219947814941, 1.2020411491394043] factorial5: recursive [1.9756920337677002, 1.9862890243530273, 1.9910380840301514, 2.0284240245819092] factorial6: reduce with lambda [2.8342490196228027, 2.8369259834289551, 2.8390510082244873, 2.8969988822937012] Arg = 10, Loops = 200000, Repetitions = 4 factorial0: math.factorial itself [0.24756813049316406, 0.24919605255126953, 0.26395106315612793, 0.28582406044006348] factorial0a: call math.factorial [0.3732609748840332, 0.37482404708862305, 0.37592387199401855, 0.38288402557373047] factorial1: for loop [0.88677501678466797, 0.89632201194763184, 0.89948821067810059, 0.90272784233093262] factorial2: reduce with op.mul [0.89040708541870117, 0.89259791374206543, 0.89863204956054688, 0.90652203559875488] factorial3: call product() [1.0093960762023926, 1.031667947769165, 1.2325050830841064, 1.7492170333862305] factorial4: while loop [0.93423891067504883, 0.93978404998779297, 0.94000387191772461, 0.95153117179870605] factorial4a: while loop with assignment operators [0.97296595573425293, 0.97462797164916992, 0.98288702964782715, 1.0095341205596924] factorial5: recursive [1.6726200580596924, 1.6786048412322998, 1.691572904586792, 1.6946439743041992] factorial6: reduce with lambda [1.8484599590301514, 1.8502249717712402, 1.8615908622741699, 1.9228360652923584] Arg = 50, Loops = 100000, Repetitions = 4 factorial0: math.factorial itself [1.6450450420379639, 1.6641650199890137, 1.6790158748626709, 1.7192811965942383] factorial0a: call math.factorial [1.7563199996948242, 2.0039281845092773, 2.1530590057373047, 2.3621060848236084] factorial1: for loop [2.7895750999450684, 2.8117640018463135, 2.8381040096282959, 3.0019519329071045] factorial2: reduce with op.mul [2.4697721004486084, 2.4750289916992188, 2.4813871383666992, 2.5051541328430176] factorial3: call product() [2.4983038902282715, 2.4994339942932129, 2.5271379947662354, 2.5356400012969971] factorial4: while loop [3.6446011066436768, 3.650169849395752, 3.6579680442810059, 3.7304909229278564] factorial4a: while loop with assignment operators [3.7421870231628418, 3.7477319240570068, 3.7655398845672607, 3.7749569416046143] factorial5: recursive [5.523845911026001, 5.5555410385131836, 5.5760359764099121, 6.2132260799407959] factorial6: reduce with lambda [4.9984982013702393, 5.0106558799743652, 5.0363597869873047, 5.0808289051055908] 

Al igual que con todos los resultados de tiempo, las entradas más rápidas en cada lista son las significativas, las más lentas deben ignorarse.

De los documentos de tiempo (cortesía de Ffisegydd ):

... el valor más bajo proporciona un límite inferior para la velocidad con la que su máquina puede ejecutar el fragmento de código dado; los valores más altos en el vector de resultados generalmente no son causados ​​por la variabilidad en la velocidad de Python, sino por otros procesos que interfieren con su precisión de tiempo. Así que el min() del resultado es probablemente el único número en el que debería estar interesado ...

Su código no entrará en el bucle while en sí. Cámbielo de n<1 a n>1 . Si, por ejemplo, encontrar un factorial de 5, n <1 se tratará como 5 <1, lo cual es falso.

 def factorial(n): result = 1 while n > 1: result = result * n n = n - 1 return result print factorial(5) 120 

Intenta esto, está más limpio:

 def factorial( n ): if n <= 1: return 1; else: return n*factorial(n-1) 

Esta es una forma recursiva de resolver el problema.

Hablando de código python corto

 def factorial(n): return reduce(lambda res, val: res*val, xrange(n, 0, -1), 1) 

Para aquellos que no entienden cómo funciona el código del volcán, aquí hay una aproximación cercana:

 ''' Equivalent code by PM 2Ring ''' def f(res, val): return res * val def factorial(n): res = 1 for val in xrange(n, 0, -1): res = f(res, val) return res