3
\$\begingroup\$

This is my palindrome-checker that is doing what it should do. If you have any additional suggestions, please let me know.

# -*- coding: utf-8 -*-


def palindrome_verif():
    Mot = raw_input("Tapez votre mot \n")

    Comptage = len(Mot)
    Lett_Sep = list(Mot)
    Decompte = 0
    Der_Lett = Comptage 

    if ((Comptage % 2) == 1):
        ImmPair = (Comptage / 2)+1
        #print(str(Decompte)," et ",str(Der_Lett))
        #print(str(Decompte)," et ",str(ImmPair))
        while Decompte < ImmPair:
            print(str(Decompte)," et ",str(Der_Lett))
            if Lett_Sep[Decompte] == Lett_Sep[Der_Lett-1]:
                print("Même lettre")
                Decompte += 1
                Der_Lett -= 1
            else:
                print("Ceci n'est pas un palindrome")
                return 0000
                #break
    elif ((Comptage % 2) == 0):
        Pair = Comptage / 2
        while Decompte < Pair:
            print(str(Decompte)," et ",str(Der_Lett))
            if Lett_Sep[Decompte] == Lett_Sep[Der_Lett-1]:
                print("Même lettre")
                Decompte += 1
                Der_Lett -= 1
            else:
                print("Ceci n'est pas un palindrome")
                return 0000
                #break
\$\endgroup\$
6
\$\begingroup\$

Writing functions

A palindrome-checking function should accept a string parameter and return a result. Your function takes its input from the keyboard (or stdin), which makes it not reusable. If you want to accept keyboard input, it should be done outside the function.

Even more unusual is the fact that you return None if it's a palindrome, and return 0000 if it's not. Why so many zeroes? Why are both return values falsy? The convention would be to return True or False.

Implementation

You're doing a lot of unnecessary things.

You can index and iterate over strings directly. There is no need to convert it into a list.

There is no need to handle odd and even lengths as separate cases.

Der_Lett (a cryptic abbreviation for "dernière lettre"?) actually isn't the index to the last letter — it's off by one. (Your code is correct, but weird.)

Style

PEP 8 naming conventions apply to your variable names, même en français. That is, they should be lowercase.

Counting loops are nearly always better done using of range(), xrange(), enumerate(), or something from itertools.

Suggested solutions

Here is a simple loop that does the job.

def verifier_palindrome(mot):
    centre = len(mot) // 2
    for decompte in range(centre + 1):
        if mot[decompte] != mot[-decompte - 1]:
            return False
    return True

Here is a more compact way to write it, using a generator expression:

def verifier_palindrome(mot):
    return all(mot[i] == mot[-i - 1] for i in range(len(mot) // 2 + 1))
\$\endgroup\$
  • \$\begingroup\$ Hi @200_success, many thanks for getting back to me. I hear you, for the variables. There will be from now on, in lowercase. I was thinking of the function's name instead -> codereview.stackexchange.com/questions/121584/… Your code , Monsieur, is much shorter than mine. I'm reading about range(), xrange() & enumerate(). Thanks again. \$\endgroup\$ – Andy K Mar 11 '16 at 6:41
  • 1
    \$\begingroup\$ Note that @YaarHever is right: my + 1 is inappropriate. In fact, it makes it crash if the input is an empty string. \$\endgroup\$ – 200_success Mar 11 '16 at 6:46
3
\$\begingroup\$

Returning True or False... literally

You should never return None or 0000 in this case. None practically is nothing and 0000 seems like False to me. For readability and sense, use True and False instead.

User input

It is best you put your raw_input statement outside the function to allow it to be reusable and can be accessed in other pieces of code. Now if the input will always be used for that one function, then you can keep the statement within the function.

Also, allow the user to type in the same line, so \n is not needed.

Extra code

There is no need to check whether the string's length is even or odd nor convert strings to lists as strings can be indexed and checked as if they were lists. Don't do unnecessary things in code: it makes it a lot more complicated.

PEP 8

If not already, use the PEP 8 convention for your code.

Shortening the code

In fact, your code can be shrunk down by a lot. By using [::-1], you can flip a string, which is needed to check whether a string is a palindrome or not. You can turn that big function into a six-liner:

def check_palindrome():
    inputted_word = raw_input("Please enter your word here: ")
    if inputted_word == inputted_word[::-1]:
        return True
    else:
        return False

Now if that is the entire program, then use print instead of return as you need to assign the returned value to a variable and print it out, which is cumbersome. With print:

def check_palindrome():
    inputted_word = raw_input("Please enter your word here: ")
    if inputted_word == inputted_word[::-1]:
        print "{0:} is a palindrome".format(inputted_word)
    else:
        print "{0:} is not a palindrome".format(inputted_word)

Note: .format() is used to put values into the string, like in this example, whatever string inputted_word is assigned to will replace {0:}.

\$\endgroup\$
  • \$\begingroup\$ As for shortening the code, why not return inputted_word == inputted_word[::-1]? \$\endgroup\$ – zondo Mar 11 '16 at 4:33
  • \$\begingroup\$ Even better: return word == reversed(word), which avoids creating a copy of the string like word[::-1] does. Note that that does twice the number of necessary comparisons in the case of a palindrome— though it's hard to argue with the readability and simplicity! \$\endgroup\$ – 200_success Mar 11 '16 at 6:50
  • 1
    \$\begingroup\$ @200_success if I do a reversed(word), I have <reversed at 0x10c8d9ed0>. Can you explain a bit more, please? \$\endgroup\$ – Andy K Mar 11 '16 at 6:58
  • 2
    \$\begingroup\$ Oops, my word == reversed(word) suggestion doesn't actually work, because reversed(word) produces an iterator, which will never be equal to a string. There's a way to make it work, but I'd rather not recommend it to a novice. Sorry for the distraction. \$\endgroup\$ – 200_success Mar 11 '16 at 7:05
2
\$\begingroup\$

200_success' generator expression seems to me the must succinct and practical. I don't understand though why adding 1 to the len is required. I would suggest the following lambda expression:

is_palindrome = lambda w: all(w[i] == w[-i - 1] for i in range(len(w) / 2))
\$\endgroup\$
  • \$\begingroup\$ Hi @yaar-hever, can you explain a bit more, please? \$\endgroup\$ – Andy K Mar 11 '16 at 6:56
  • \$\begingroup\$ Lambda expressions are small anonymous functions that get an argument and return a value. They can be assigned to a variable like any other expression. \$\endgroup\$ – Yaar Hever Mar 11 '16 at 19:50
  • \$\begingroup\$ Regarding the + 1 issue, you can just count and see: A palindrome like "abccba" has 6 characters, so len(w) / 2 gives 3 and range(3) gives [0, 1, 2]. A palindrome like "abcdcba" has 7 characters, but len(w) / 2 is still 3 (rounded down, because it's an integer). The middle character 'd' doesn't need to be compared on both sides of the string, since it only appears once. \$\endgroup\$ – Yaar Hever Mar 11 '16 at 19:57
1
\$\begingroup\$

You are overkilling it. Please, use a proper separation of tasks: do not ask for input in the method that should only check whether the input string is a palindrome.

All in all, I had this in mind:

def is_palindrome(str):
    left = 0
    right = len(str) - 1

    while left < right:
        if str[left] != str[right]:
            return False

        left += 1
        right -= 1

    return True

word = input("Type in a string: ")

print(word, " is a palindrome: ", is_palindrome(word))

Hope this helps.

\$\endgroup\$
  • \$\begingroup\$ Hi @coderodde, many thanks for getting back to me. What do you mean by do not ask for input in the method? What is the method here, in the function I wrote? I'm learning , therefore I'm asking sometimes stupid questions. \$\endgroup\$ – Andy K Mar 11 '16 at 6:43
  • \$\begingroup\$ There is no stupid questions. However, the point is this: in each method try to solve only 1 problem. \$\endgroup\$ – coderodde Mar 11 '16 at 6:48
  • \$\begingroup\$ So the function, I presented, in the scenario, should solve one problem. I got it. Cheers. \$\endgroup\$ – Andy K Mar 11 '16 at 6:59
1
\$\begingroup\$

A cleaner solution would be to cut the string in half, reverse the second half, then compare those. Something like this:

def palindrome_verif(string):
    firstpart, secondpart = string[:math.ceil(len(string)/2)],   string[len(string)//2:]
    return firstpart == secondpart[::-1]

word = input("Type in a string: ")

print(word, " is a palindrome: ", palindrome_verif(word))
\$\endgroup\$

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.