4
\$\begingroup\$

I'm a rank beginner in Python, and I am working my way through various relevant OpenCourseware modules. In response to the prompt

Write a procedure that takes a list of numbers, nums, and a limit, limit, and returns a list which is the shortest prefix of nums the sum of whose values is greater than limit. Use for. Try to avoid using explicit indexing into the list.

I wrote the following simple procedure

def numberlist(nums,limit):   
    sum=0  
    i=0  
    for i in nums:  
        sum=sum+i  
        if sum>limit:  
            return i  
        else:  
            print i

It gets the job done, but the division of labor between if and else seems inelegant, as does the use of both return and print. Is there a better way to structure this basic loop?

\$\endgroup\$
  • \$\begingroup\$ I see now that this version was cobbling together a numberlist from the output of print and return, which would be problematic if I tried to pass the result into another function. Thanks to the commenters pointing this out! \$\endgroup\$ – K. Olivia Jun 22 '12 at 7:24
9
\$\begingroup\$

So, a couple things:

  1. The problem statement says nothing about printing data, so you can omit the print statement, and thus the entire else: clause, entirely.

  2. The problem statement says to return a list, and you're just returning the last item in that list, not the entire list.

Here's a short but inefficient way to do it:

def numberlist(nums, limit):
    i = 0
    while sum(nums[:i]) < limit:
        i += 1
    return nums[:i]

or a more efficient but longer way:

def numberlist(nums, limit):
    prefix = []
    sum = 0
    for num in nums:
        sum += num
        prefix.append(num)
        if sum > limit:
            return prefix
\$\endgroup\$
  • \$\begingroup\$ I didn't realize that the question was asking for the subset. Have an upvote :) \$\endgroup\$ – rahul Jun 20 '12 at 2:48
  • \$\begingroup\$ This, like blufox’ solution, uses indexing which we’ve been told to avoid. \$\endgroup\$ – Konrad Rudolph Jun 20 '12 at 12:26
  • \$\begingroup\$ Konrad: the inefficient way does, the other way doesn't. \$\endgroup\$ – pjz Jun 20 '12 at 15:46
  • \$\begingroup\$ pjz: what makes the second method more efficient, in your estimation? \$\endgroup\$ – K. Olivia Jun 22 '12 at 7:27
  • \$\begingroup\$ K. Olivia: the first one runs a sum on the whole slice every time through the loop. The second only adds the newest number to the sum, which is much faster. \$\endgroup\$ – pjz Jun 22 '12 at 14:07
6
\$\begingroup\$

Try to avoid using explicit indexing into the list.

This part in the question was ignored in the other (good) answers. Just to fix this small shortcoming, you can write a generator which avoids indexing completely:

def numberlist(nums, limit):
    sum = 0
    for x in nums:
        sum += x
        yield x
        if sum > limit:
            return

This will return an iterator that, when iterated over, will consecutively yield the desired output:

>>> for x in numberlist([2, 4, 3, 5, 6, 2], 10):
...     print x,
... 
2 4 3 5

However, strictly speaking this violates another requirement, “returns a list” – so we need to wrap this code into another method:

def numberlist(nums, limit):
    def f(nums, limit):
        sum = 0
        for x in nums:
            sum += x
            yield x
            if sum > limit:
                return

    return list(f(nums, limit))
\$\endgroup\$
5
\$\begingroup\$

The others have discussed how you aren't quite doing what the problem asks, I'll just look at your code:

def numberlist(nums,limit): 

When the name of a function has two words it in, we recommend separate it with an _, in this case use number_list. Its easier to understand the name

    sum=0  

sum is the name of a built-in function, you should probably avoid using it

    i=0  

This does nothing. You don't need to pre-store something in i, just use the for loop

    for i in nums:  

I really recommend against single letter variable names, it makes code hard to read

        sum=sum+i  

I'd write this as sum += i

        if sum>limit:  

I'd put space around the >

            return i  
        else:  
            print i

Your instinct is right, using both return and print is odd. As the others have noted, you shouldn't be printing at all.

\$\endgroup\$
  • \$\begingroup\$ I generally agree about the one-letter variable names but there’s a broad consensus that they’re OK as simple loop variables (among others) – even though i is conventionally reserved for indices … \$\endgroup\$ – Konrad Rudolph Jun 20 '12 at 12:33
  • \$\begingroup\$ @KonradRudolph, I disagree with the consensus. But yes, there is one. \$\endgroup\$ – Winston Ewert Jun 20 '12 at 15:16
2
\$\begingroup\$

Welcome to programming :) I did not understand your question first, then I realized that python might be your first language. In that case congratulations on picking a very nice language as your first language.

Your question seems to ask for the list which is the shortest prefix of nums the sum of which is greater than the limit. Here, you might notice that it does not care about the intermediate values. Alls that the function asks is that the return value be greater than the limit. That is, this should be the output

>>> numberlist([1,2,3,4,5], 5)
[1,2,3]

No output in between. So for that goal, you need to remove the print statement in your code, and without the print, there is no need for the else. In languages like python, it is not required that there be an else section to an if-else conditional. Hence you can omit it. We can also use enumerate to iterate on both index and the value at index. Using all these we have,

def numberlist(nums,limit):   
    sum=0  
    for index,i in enumerate(nums):  
        sum += i
        if sum>limit:  
            return nums[:index+1]

Note that if you are unsatisfied with omitting the else part, you can turn it back by using pass. i.e

    if sum>limit:  
        return nums[:index+1]
    else:
        pass

Note also that I used an array slice notation nums[:index+1] that means all the values from 0 to index+1 in the array nums This is a rather nice for loop. If you are feeling more adventurous, you might want to look at list comprehensions. That is another way to write these things without using loops.

edit: corrected for enumeration

\$\endgroup\$
0
\$\begingroup\$

I just started programming in Python (well in general) a week ago

This is the analogy I used to make sense of the problem.

You have a word like Monday. Each character in the word has a value: 'M' = 1, 'o' = 2, n = '3', d = '4', a = '5', y = '6'. If you added the value of each character in the word 'Monday' it would be: 1 + 2 + 3 + 4 + 5 + 6 = 21 So the 'numerical value' of Monday would be 21

Now you have a limit like 9

The question is asking you to create a program that takes a list of numbers like [1, 2, 3, 4, 5, 6] And find out how many of these numbers (starting from the left because prefix means the beginning of word or in this case the list – which starts from the left i.e. 1) can be added together before their sum is greater than the limit. The answer would be the sum of the numbers 1, 2, 3 and 4 which is 10. Your prefix is the list [1, 2, 3, 4]

num = [1,2,3,4,5,6]

limit = 9

def prefixLimit(limit, num):
    sumOfPrefix = 0
    prefix = [] 
    for i in num:
        if sumOfPrefix < limit:
            sumOfPrefix = sumOfPrefix + i
            prefix.append(i)
            if sumOfPrefix > limit:
                return prefix

print(prefixLimit(limit, num))
\$\endgroup\$
  • \$\begingroup\$ This is not a review of the provided code. At Code Review, we expect all answers to be reviews. \$\endgroup\$ – Mast Sep 1 '16 at 12:47
  • \$\begingroup\$ I would appreciate clarity as to what I did wrong. I'm new to this - forum, programming - any help will be greatly beneficial to my progress \$\endgroup\$ – The Known Sep 3 '16 at 11:16
  • \$\begingroup\$ Sure, take a look at the help/how-to-answer. \$\endgroup\$ – Mast Sep 3 '16 at 16:06
-2
\$\begingroup\$

I would use following simple function with while loop:

numberlist(nums,limit):  
  nums =[]
  i=1

   partial_sum=sum(nums)
   while partial_sum< limit:
       nums.append(i)
       i+=1
       partial_sum=sum(nums)
   return partial_sum
New contributor
Krzysiek Bienias is a new contributor to this site. Take care in asking for clarification, commenting, and answering. Check out our Code of Conduct.
\$\endgroup\$

We are looking for answers that provide insightful observations about the code in the question. Answers that consist of independent solutions with no justification do not constitute a code review, and may be removed.

  • 1
    \$\begingroup\$ You have presented an alternative solution, but haven't reviewed the code. Please edit to show what aspects of the question code prompted you to write this version, and in what ways it's an improvement over the original. It may be worth (re-)reading How to Answer. \$\endgroup\$ – Toby Speight yesterday

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.