[eugenerator]

I'm working on a function that checks to see if a number is a palindrome or not. Here is what I have done:

def palindrome(number):
    palindrome = True
    numString = str(number) #converts into string
    numList = list(numString) #converts into list so the reverse function can be used
    numRev = numList.reverse()
    
    if(numList[0] == numList[len(numList)-1]): #Checks to see if the first digit is the same as the last 
        for i in range(1, len(numList)- 2): #Checks the inside digits to see if they match up
            if (numList[i] != numRev[i]):
                palindrome = False #if one of them doesn't, it's not a palindrome
                break
    else: #If the first and last digits aren't the same, it isn't a palindrome
        palindrome = False
    return palindrome #Returns 'True' or 'False' value..

I think my function is logically correct (If not, tell me!). It works perfectly with 3 digit numbers, but when I try to use numbers any longer than that, it gives me the following error message:

  File "<pyshell#26>", line 9, in palindrome
    if (numList[i] != numRev[i]):
TypeError: 'NoneType' object is not subscriptable

I've looked up what this error message means, but..I don't understand what I need to do to fix it. Help?

Thanks!

Also, I'm fairly new to this stuff, so don't judge my silly questions!

[sepp2k]

The error message is telling you that either numList or numRev are None instead of being a list. So we should look at the last time either of them is assigned.

numList is assigned only on line 4. Its value is the result of list(numString). list always returns a list, so numList must be a list.

numRev is assigned on line 5. Its value is the result of numList.reverse(). reverse is an in-place operation that returns None. So numRev is None.

You might want to use the reversed function rather than the reverse method. reversed returns a new iterator instead of modifying its argument in-place. Note that the iterator returned by reversed, needs to be converted to a list first if you want to use it as a list.

PS: If you want to see if two lists contain the same elements, you can just use == - no need to iterate over them manually.

This post has been edited by sepp2k: 22 June 2012 - 06:36 PM

[atraub]

list.reverse() reverses a list in place, it does not return a new list. A minor change will fix that.

Replace

numRev = numList.reverse()

with

    numRev = list(numList)
    numRev.reverse()

EDIT:
I completely forgot about the "reversed" keyword, good call.

This post has been edited by atraub: 22 June 2012 - 06:40 PM

[eugenerator]

Okay guys, thank you! That was very helpful.

[eugenerator]

I actually have one other problem..

So I went in and finished my program using the Palindrome function. However, one of the numbers that ran through the function was 990009, and it registered as 'true'

I looked through the code like 20 times and I don't understand how this could be happening

[sepp2k]

range is exclusive, i.e. the sequence returned by range(n) will not include n. So range(1, len(numList)- 2) will not give you the index for the second to last element, so that is never checked.

PS: Is there a reason why you compare the first and last character outside the loop, instead of just looping over all the characters?

[eugenerator]

sepp2k, on 22 June 2012 - 06:51 PM, said:

range is exclusive, i.e. the sequence returned by range(n) will not include n. So range(1, len(numList)- 2) will not give you the index for the second to last element, so that is never checked.

PS: Is there a reason why you compare the first and last character outside the loop, instead of just looping over all the characters?

I actually adjusted the range(1, len(numList)- 2), but it still says that 990009 is a palindrome. Here is all of the code:

def palindrome(number):
    palindrome = True
    numString = str(number)
    numList = list(numString)
    numRev = numList
    numList.reverse()
    
    if(numList[0] == numList[len(numList)-1]):
        for i in range(0, len(numList)- 1):
            if (numList[i] != numRev[i]):
                palindrome = False
                break
    else:
        palindrome = False
    return palindrome            

num1 = 999
num2 = 999

largestPal = 0
while(num1 > 100):
    while(num2 > 100):
        if(palindrome(num1*num2) == True):
            if((num1*num2) > largestPal):
                largestPal = num1*num2
                print(largestPal)
        num2 = num2-1
    num1 = num1-1

And to answer your PS, the intention was to save time. Why check numbers whose first and last digits aren't the same?

[sepp2k]

That doesn't really save time though. If you didn't have the if, such numbers would be found in the first iteration of the loop anyway. So it's one check either way.

Anyway, your problem is that numList and revList in your code point to the same list because you didn't create a copy of numList when assigning it to revList like atraub did. Personally I prefer to avoid mutating methods exactly to avoid issues like this.

[Dogstopper]

I understand your approach, but why not just compare the normal string with the revered string?

>>> string = str(990992)
>>> rev_list = list(string)
>>> rev_list.reverse()
>>> rev_str = "".join(rev_list)
>>> string == rev_str
False

Or, you could iterate within itself...

def Test(string):
	i = 0
	while len(string)/2 > i:
		if string[i] != string[-(i+1)]:
			return False
		i=i+1
	return True

[eugenerator]

Oh, I see. Yeah, that is a bit redundant on my part. I stand corrected!

*le sigh*

Why am I so bad?

[atraub]

Yeah, last night I wrote my own version too:

def palindrome(number):
    return str(number) == ''.join([i for i in reversed(str(number))])