4
\$\begingroup\$

I am beginning to learn about object oriented programming in my programming class, and to make sure I had all the ideas down, I made a small palindrome checker program. It works by checking if the letter on the opposite side is the same.

from time import sleep  # To add readability delays


class PalindromeFinder:

    # Makes the variables
    def __init__(self):
        self.letter_count = {}
        self.loop = 0

    def check_for_palindrome(self, base):
        self.loop = 0
        for letter in base:
            # Checks if the letter in x position is the same as the letter in the -x position
            if base[self.loop] != base[-(self.loop + 1)]:
                return (f"{base} is not a palindrome")
            self.loop += 1
        return (f"{base} is a palindrome.")


def introduce_palindromes():
    print("Palindromes are really cool!")
    sleep(2)
    print("It when a word or phrase is the same backwards and forwards!")
    sleep(2)
    print("For example", palindrome_checker.check_for_palindrome("racecar"))
    sleep(2)
    print("While", palindrome_checker.check_for_palindrome("alarm"))
    sleep(2)
    print("Now you enter some palindromes!")


palindrome_checker = PalindromeFinder()
introduce_palindromes()
while True:
    sleep(1)
    print(palindrome_checker.check_for_palindrome(input("Enter your palindrome: ").lower()))

Since I have no idea what I am doing, I just generally want to know if what I am doing is not violating any conventions. More specifically, I want to know if having both functions and classes are ok, as well as if putting the variables in __init__ like I did is fine. Also, when initializing several instances at once, is there any documentation on were you do it? Do you do it between the class and functions or between the functions and the code?

Any other advice is welcome.

\$\endgroup\$
  • 1
    \$\begingroup\$ As you stated "I have no idea what I am doing", all your effort is replaced with a short function def is_palindrome(inp_text): return inp_text == inp_text[::-1]. That's all \$\endgroup\$ – RomanPerekhrest Feb 26 at 20:06
  • 3
    \$\begingroup\$ OOP isn't necessarily the best tool for every job, so this specific thing might not be a good example to learn with. Something that involves multiple steps and state that needs to be tracked between steps would be a better use case for an object. \$\endgroup\$ – Samwise Feb 26 at 22:14
4
\$\begingroup\$

As stated in the comments, this is not a good example for OO programming, because it can be solved with just a function. Sometimes you can still use classes to separate different functionality, but in Python this is not really required nor generally practiced (compared to e.g. Java where all code must be in a class - if we discount default methods in interfaces, static code blocks and such).

Now you've put designated two variables as fields, while they should be local to the function. There is no need at all to use them as fields. This won't directly show up as a problem, but if you'd e.g. call the method check_for_palindrome in a concurrent fashion, you'd be in trouble, because the loop field will now be used by two different threads at once. letter_count on the other hand is secure, but only because it is simply not used.

Only create fields for variable values that are intrinsic to the class (or are references that tie the class to another of which it is dependent). For instance, you might want to configure the checker to check for upper or lowercase, or maybe even mixed case. That kind of field would be logical for a palindrome checker class. All in all, you should try and minimize the variables that make up the state and thus number & complexity of the fields for a particular class.

As you've already found out yourself, PalindromeChecker would be a better name for the class, as it doesn't search for palindromes, at least not at the moment. If you've got a method for a class, you don't need to repeat everything, just PalindromeChecker.check would be sufficient. I don't know why the input is called base, just input seems sufficient (edit: word would probably even be a better name).

When it comes to functionality of a class, it is important to distinguish human interaction with actual output. I'd never create a checker that would return a string as response. In this case I'd expect a bool output instead. Then the program that uses the checker can create a user response if the result is True or False. At least didn't print out the result, which would have been the action of most starting developers.

Finally, in the call to the class instance, you're trying to do too much at once:

print(palindrome_checker.check_for_palindrome(input("Enter your palindrome: ").lower()))

First assign the result to a variable and then print it out. Separate the input. You'll thank yourself if you have to startup a debugger and step through your program.

| improve this answer | |
\$\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.