python function returns true when it should return false-CodePudding

I am trying to write a function to check whether a string is palindrome or not, but every string is showing as palindrome

def is_palindrome(input_string):    
    x=0
    reverse_string = ""
    while x<len(input_string):
        reverse_string =input_string[x]
        x=x 1    
    if input_string == reverse_string:
        return True
    else:
        return False
    


print(is_palindrome("abc")) # Should be False but it return True

CodePudding user response：

In your code, the variabile "reverse_string" will always be equal to "input_string" since you are just appending the characters in the same order with the = operator.

A simple way to reverse a string in Python is to use slicing like that:

def is_palindrome(input_string):    
    if input_string == input_string[::-1]:
        return True
    return False

input_string[::-1] means "start from the first index to the last in the reverse order (-1)"

CodePudding user response：

Your problem is in the reversal of the string. (your x is going from 0 to len(input_string)-1 but it should go the other way)

That's why it's important to break code into functions that do one and only one thing (at least in the beginning)

In this case is an overkill, but it will help you when your code grows more complex.

your function can then be simplified as:

def is_palindrome(input_string): 
    return input_string == reverse_string(input_string)

If you look at it is self explanatory. Is the input string equal to its reverse?

Now we need to implement the function reverse_string.

The advantage of having a function that just reverses a string is that we can do a lot of tests on it to check just this particular function

In your case, you can use negative indexes, or you can start with the index set to len(input_string)-1 and go towards 0.

But it's also a good moment to learn about string slicing and how to do things in a pythonic way, so the reverse function can be written as:

def reverse_string(input_string):
    return input_string[::-1]

Feel free to put your own implementation of reverse_string if you are not yet confident with string slicing, but with this function you have separated two different things: reversing a string and checking is string is a palindrome. You can even reuse that reverse_string function later on.

Now we can test it with many cases until we are confident that it works as expected.

I'd recommend taking a look at unit tests it might seem too much for such an easy problem, but it will help you a lot in the future.

Just test what happens if you pass a palindrome, a non-palindrome, an empty string, a number, a None...