15
\$\begingroup\$

I recently did a little task for fun that required me to create a class to determine if a word is a palindrome or not. I know this is quite simple, but I'd be interested to know if a real developer (I'm not a programmer) would approach the task in the same way, or if I make any amateur mistakes/do anything stupid.

My code passed the automated tests on the website, but I understand those aren't perfect.

class Palindrome
{
    public static function isPalindrome($word)
    {
        $word = strtolower($word);
        $wordLength = strlen($word);
        $wordSplitPoint = ceil($wordLength / 2);

        if ($wordLength % 2 == 0) {
            $firstHalf = substr($word, 0, $wordSplitPoint);
            $secondHalf = substr($word, $wordSplitPoint, $wordSplitPoint);
        } else {
            $firstHalf = substr($word, 0, $wordSplitPoint-1);
            $secondHalf = substr($word, $wordSplitPoint, $wordSplitPoint);
        }

        $secondHalfReversed = strrev($secondHalf);

        if ($firstHalf == $secondHalfReversed) {
            return TRUE;
        } else {
            return FALSE;
        }
    }
}

echo Palindrome::isPalindrome('Deleveled');
\$\endgroup\$
2
  • 5
    \$\begingroup\$ Perhaps this is "outside" of the question/problem, but: Why is it in a class? Why not just make a standalone function? I don't know PHP, but to my understanding, it doesn't mandate top-level classes like Java does. \$\endgroup\$
    – Quelklef
    May 22, 2018 at 2:49
  • 1
    \$\begingroup\$ @Quelklef The task specifically asks for it to be in a class and only allows you to modify what is contained within the isPalindrome function \$\endgroup\$
    – Matadeleo
    May 22, 2018 at 9:07

5 Answers 5

36
\$\begingroup\$

Others have pointed out a fix you could make to your implementation. But there is a way to write this which is much simpler. That takes advantage of a simplified definition of palindrome: "a word which reads the same forwards and backwards".

Now I haven't done PHP in years, and don't have a test environment, so this might have issues, but the basic idea should be clear.

public static function isPalindrome($word)
{
    return $word == strrev($word);
}

Update

Based on suggestions in the comments, this would be nicer:

function isPalindrome($word) {
    $word = preg_replace('/[^a-zA-Z]/', '', $word);
    $word = strtolower($word);
    return $word == strrev($word);
}

That will now correctly identify "Level" (ignoring capitalization) and "Madam, I'm Adam." (ignoring spaces and punctuation.)

\$\endgroup\$
7
  • \$\begingroup\$ Smart :) I like it! \$\endgroup\$
    – Matadeleo
    May 21, 2018 at 22:12
  • \$\begingroup\$ And what is the reason of the divorce? \$\endgroup\$ May 22, 2018 at 10:53
  • 3
    \$\begingroup\$ You should make $word lowercase before you do the return. Else "Level" will not be valid as a palindrome. \$\endgroup\$
    – Tonny
    May 22, 2018 at 11:26
  • 1
    \$\begingroup\$ this is exactly what i was thinking :) I agree with Tonny, but would also add something to strip white spaces so that "taco cat" evaluates as true \$\endgroup\$
    – Doug
    May 22, 2018 at 12:58
  • 3
    \$\begingroup\$ @BillalBEGUERADJ: I never did much with it. But I absolutely prefer functional languages: ML, Haskell, OCaml, Erlang, various LISPs, and usually program by day in JavaScript. \$\endgroup\$ May 22, 2018 at 14:13
8
\$\begingroup\$

This is a very good program. There is only one thing I'd change:

if ($firstHalf == $secondHalfReversed) {
    return TRUE;
} else {
    return FALSE;
}

You don't really need that if because the top part only returns TRUE when the condition also is TRUE. Otherwise, both the condition and the return is FALSE. Change it to:

return $firstHalf == $secondHalfReversed
\$\endgroup\$
8
\$\begingroup\$

To start with id remove the code that you written twice by re-arranging the if statement into this

if ($wordLength % 2 == 0) {
    $firstHalf = substr($word, 0, $wordSplitPoint);
} else {
    $firstHalf = substr($word, 0, $wordSplitPoint-1);
}

$secondHalf = substr($word, $wordSplitPoint, $wordSplitPoint);

Then for the return value I would use;

return $firstHalf == $secondHalfReversed;

This is about all i would change

\$\endgroup\$
1
  • \$\begingroup\$ Thanks, no reason why the $secondHalf should be repeated, and that return is much cleaner \$\endgroup\$
    – Matadeleo
    May 21, 2018 at 18:35
2
\$\begingroup\$

I'd write this using a traditional for loop as follows:

class Palindrome
{
    public static function isPalindrome($word)
    {
      $wordLength = strlen($word)-1;

      for ($i = 0; $i < $wordLength/2; $i++) {
        if (strtolower($word[$i]) != strtolower($word[$wordLength-$i])) {
          return FALSE;
        }
      }

      return TRUE;
    }
}

echo Palindrome::isPalindrome('Deleveled');
\$\endgroup\$
2
  • 1
    \$\begingroup\$ Pedantically, this approach would be less computing power than the other approaches. \$\endgroup\$
    – Kenneth K.
    May 22, 2018 at 13:17
  • \$\begingroup\$ You have presented an alternative solution, but haven't reviewed the code. Please edit to show what aspects of the question code prompted you to write this version, and in what ways it's an improvement over the original. It may be worth (re-)reading How to Answer. \$\endgroup\$ Jan 23, 2019 at 17:23
1
\$\begingroup\$
echo Palindrome::isPalindrome('Deleveled');

You need to test corner cases and try to break your code. For example, you might try with:

  • a string of odd size
  • a string of even size
  • the empty string
  • a string with one character

How is size defined anyway? What about encodings? For example, your code works for '101', but not 'é_é'?

\$\endgroup\$
1
  • \$\begingroup\$ The task I completed on the website wanted a function that could detect single-word palindromes of varying lengths. But you're right. Edge cases are something I need to consider more. I'm trying hard to leave the world of "duck-tape" php behind. "It only broke because you aren't using it correctly" is a terrible philosophy :) \$\endgroup\$
    – Matadeleo
    May 23, 2018 at 9:14

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.