2 
def divisors(n):
    ans =0
    for i in range(n):
        i += 1
        k=n%i
        if k!=1:
            ans=i
    return ans
print(20)

I have a function that's not working properly when I run it prints the n value instead of printing out the divisors.

Improve this question
4
  • What is the result you want? Commented Oct 10, 2015 at 15:46
  • so it should be printing out the divisors for 20 which would be 1,2,4,5,10,20 Commented Oct 10, 2015 at 15:47
  • 1
    The code has not called the def() function. It prints "20" just like you told it to. You might try print(divisors(20)), but that will only print the last ans value since divisors() does not return a list. Commented Oct 10, 2015 at 15:54
  • 1
    @AndreL I've rolled back the edit you made for a couple of reasons. The first and biggest was that your change to the code invalidated part of a legitimate answer and was not based on comments provided by the OP. If the code does not do what the OP claims it does, you may inadvertently hide the problem by making those sorts of changes to the code. Additionally, the other changes you made did not improve the post. It moved some text to a different place, but there were clear, obvious, low effort to fix grammatical and mechanical changes you could have made. Commented Oct 11, 2015 at 1:25

4 Answers 4

Reset to default
8

Three key issues are:

  1. k=n%i returns the remainder - if the remainder != 1 it doesn't mean that i is a divisor!
  2. in the for-loop you keep overriding ans and at the end of the function you return the value that was found on the last iteration that satisfied the if condition. What you want to do instead is to accumulate all the anss into a list and return that list.
  3. The print at the end is not calling the function - it simply prints the number 20.

I'm not posting a corrected solution because I think that it will be a good exercise for you to fix these bugs by yourself, good luck!

Improve this answer
Sign up to request clarification or add additional context in comments.

4 Comments

def divisors(n): ans = [] for i in range(n): i += 1 if n % i == 0: ans.append(i) return ans
@user4574134 you got it :)
def divisors(n): ans = [] for i in range(n): i += 1 if n % i == 0: ans.append(i) return ans print(divisors([30])) this is not working when i enter 30 as a list value
@user4574134 the function receives an integer as an argument, why would you pass a list ?
3

I saved the result in a list:

def divisors(n):
    ans = []
    for i in range(n):
        i += 1
        k=n%i
        if k==0:
            ans.append(i)
    return ans

print divisors(20)
Improve this answer

1 Comment

You missed something when cleaning your code. The function should return ans.
2

If you what a more elegant way you can use lists-comprehension, also you reduce it in one line.

def divisors(n):
    return [i+1 for i in range(n) if n%(i+1) == 0]
Improve this answer

Comments

1

Try to use generator function with yield for that purpose:

def divisors(n):
    iterable = xrange(1, n + 1)
    for i in iterable:
        k = n % i
        if k == 0:
            ans = i
            yield ans

print list(divisors(20))
Improve this answer

1 Comment

i think that is an advanced approach for the OP level, but its a good answer indeed.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.