Simple palindrome function for single words

Question

I have created a simple palindrome function that works only on single words. It is working as intended, but I feel that it definitely needs improvement.

def is_palindrome(w):
 if not isinstance(w, str):
 return "ERROR: input is not a string."
 elif len(w) > 2:
 if w[0] == w[-1]:
 while len(w) > 2:
 w = w[1:-1]
 is_palindrome(w)
 if len(w) == 1:
 return True
 else:
 return False
 else:
 return "ERROR: input contains two or less characters."
print is_palindrome('aibohphobia')
print is_palindrome('definitely')

Can you please review my function and post your suggestions?

Also, is it better to do w[:] = w[1:-1] instead of w = w[1:-1]?

Answer 1

Firstly, your testing is insufficient. This code thinks that "defad" is a palindrome and that "deed" is not. You algorithm is completely broken.

def is_palindrome(w):
 if not isinstance(w, str):
 return "ERROR: input is not a string."

Don't check types. Just treat it like a string, and trust the caller to pass the right kind of object. Also don't return strings as errors, throw exceptions.

 elif len(w) > 2:
 if w[0] == w[-1]:
 while len(w) > 2:
 w = w[1:-1]
 is_palindrome(w)

You call the function but you don't do anything with the result.

 if len(w) == 1:
 return True
 else:
 return False

This is equivalent to: return len(w) == 1

 else:
 return "ERROR: input contains two or less characters."

Why can't palindromes be zero or one characters? Also, throw exceptions, don't return strings.

Answer 2

Why don't you check this way?

def is_palindrome(w):
 return w == w[::-1]

Answer 3

San4ez has the best implementation, and Winston Ewert raised valid points. I'd still like to add a few comments.

To answer your question, you should not use w[:] = w[-1] on strings, since this means trying to change your string, and strings are immutable in Python. w = w[-1] simply creates a new string, also named w.

Note than recursive functions are not Pythonic, since there is no tail call optimization, which means it would have been impossible for your recursive call to be optimized anyway. A simpler implementation:

def is_palindrome(w):
 while w and w[0] == w[-1]:
 w = w[1:-1]
 return w == ''

If you really wanted to go for the recursive way, you could have written:

def is_palindrome(w):
 return w == '' or w[0] == w[-1] and is_palindrome(w[1:-1])

• lvc: I have to disagree that recursive functions in general aren't pythonic. For example, I would certainly argue that your recursive version reads more readily than your iterative version, although writing it as return True if w == '' else w[0] == w[-1] and is_palindrome(w[1:-1]) could be a little clearer still (though that's debatable). Certainly, when you say recursion isn't Pythonic because Python doesn't do tail-call optimisation, I have to point out that's only CPython, and it doesn't matter here anyway since the OP as written doesn't tail recurse.
• Quentin Pradet: @lvc it's a very interesting discussion. I do prefer the recursive solution since I have used functional languages a lot. I'm not sure it reads easily if you never have written a recursive function before. CPython is the default and most used implementation, and it makes sense to always consider it when writing code. There are a lot of ways to write "functional" code in Python, but Guido van Rossum still chose not to support TCO since it's "unpythonic". Whenever you have the choice, iterative code seems more pythonic.
• Quentin Pradet: It's only about using "idioms" in the language. It's worth to mention in a code review, but you can still write "functional-like" code in your mind, bearing in mind that writing a tail recursive function won't help at all and will only hurt readability.

Answer 4

You might try this function:

def is_palindrome(text):
 return text[:len(text)//2] == text[:(len(text)-1)//2:-1]

Here is example usage for reference:

>>> is_palindrome('')
True
>>> is_palindrome('a')
True
>>> is_palindrome('b')
True
>>> is_palindrome('aa')
True
>>> is_palindrome('ab')
False
>>> is_palindrome('ba')
False
>>> is_palindrome('bb')
True
>>> is_palindrome('aaa')
True
>>> is_palindrome('aab')
False
>>> is_palindrome('aba')
True