Rosamunda
Rosamunda

Reputation: 14980

Python: Why this tuple is printing the result and "None"? Also: Is there a better way to achieve the result?

I need to calculate with a given number, how many "fives", "twos", and "ones" can get into those numbers. Sorry my english is a little limited for this sort of explanation :) Maybe an example is better:

Excercise: print stamps(8) The result should be: (1, 1, 1) ( one 5p stamp, one 2p stamp and one 1p stamp)

I´ve found the way to achieve that, but the tuple() is printing the result and "None", and I don´t know why. I also would like to know is there´s a better, shorter way to get to the correct result.

This is what I´ve done:

def stamps(dinero):
    p5=dinero/5
    p5a=p5*5
    resultado1=dinero-p5a
    dinero=resultado1
    p2=dinero/2
    p2a=p2*2
    resultado2=dinero-p2a
    dinero=resultado2
    p1=dinero/1
    p1a=p1*1
    resultado3=dinero-p1a
    dinero=resultado3
    print tuple([p5,p2,p1])

The result that I get with: print stamps(8) is (1, 1, 1) None

Update: I´ve found a better solution, I´m posting it here just in case anyone wonders for a better solution:

def stamps(n):
    #Basically, thats the same as return n/5, n%5/2, n%5%2
    return n/5, (n-5*(n/5))/2, (n-5*(n/5))-2*((n-5*(n/5))/2)

Upvotes: 1

Views: 473

Answers (2)

user764357
user764357

Reputation:

As people have stated you can change the print to a return, however the big improvement to your code is to use the % (or modulo) operator.

def stamps(dinero):
    p5=dinero/5
    dinero=dinero%5
    p2=dinero/2
    dinero=dinero%2
    p1=dinero/1
    return tuple([p5,p2,p1])

print stamps(8)
>>> (1,1,1)

In your code this line:

p5=dinero/5

Performs integer division, while the below gets the remainder, by multiplying the number of multiples of the divisor in the original number and subtracting it:

p5a=p5*5
resultado1=dinero-p5a
dinero=resultado1

Most languages offer a modulo function that performs this in a single step:

dinero=dinero%5

This is the same for the part where you divide by 3, and when you divide by 1 there is never an integer remainer, so you can remove that code completely.

Python also has a way that you can shorten this again using divmod() which returns both the divisor and modulus:

def stamps(dinero):
    p5,dinero=divmod(dinero,5)
    p2,dinero=divmod(dinero,2)
    p1=dinero
    return tuple([p5,p2,p1])

print stamps(8)
>>> (1,1,1)

And lastly, you can generisise it completely, by having another function take both the amount and an array of stamp values and call that:

def stamps(dinero):
    return allStamps(dinero,[5,2,1])

def allStamps(dinero=1,stamps=[]):
    vals = []
    for stamp in sorted(list(set(stamps)), reverse=True):
        val,dinero=divmod(dinero,stamp)
        vals.append(val)
    return tuple(vals)

print stamps(8)
>>> (1,1,1)
print allStamps(8,[5,3,1])
>>> (1,1,0)

Regarding code execution speed:

I ran a timeit on some of the options, and the calls to / and % turned out faster than even a single call to divmod():

> python -m timeit 'a=1000;b=a/5;c=b*5;d=a-c;a=d'
 10000000 loops, best of 3: 0.156 usec per loop
> python -m timeit 'a=1000;b=a/5;a=a-b*5;'
 10000000 loops, best of 3: 0.127 usec per loop
> python -m timeit 'a=1000;a=a-(a/5)*5;'
 10000000 loops, best of 3: 0.121 usec per loop
> python -m timeit 'a=1000/13;b=1000%13;'
 10000000 loops, best of 3: 0.0755 usec per loop
root@meteordev:~# python -m timeit 'a,b=divmod(1000,13);'
 10000000 loops, best of 3: 0.183 usec per loop

Upvotes: 2

user2566092
user2566092

Reputation: 4661

Change "print" to "return" inside your function and that should fix it when you call "print stamps(8)". Also, no, for your simple choice of stamp values 5,2,1, there is no more efficient way to find a good solution other than what you're doing (the only possible improvement would perhaps to use a for loop if you had more stamp values than just 3) -- if your stamp values were more complex, then you could find better solutions that use fewer stamps by using dynamic programming.

Upvotes: 0

Related Questions