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\$
  • 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 '18 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 '18 at 9:07
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\$
  • \$\begingroup\$ Smart :) I like it! \$\endgroup\$ – Matadeleo May 21 '18 at 22:12
  • \$\begingroup\$ And what is the reason of the divorce? \$\endgroup\$ – Billal Begueradj May 22 '18 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 '18 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 '18 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\$ – Scott Sauyet May 22 '18 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\$
  • \$\begingroup\$ Thanks, no reason why the $secondHalf should be repeated, and that return is much cleaner \$\endgroup\$ – Matadeleo May 21 '18 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\$
  • 1
    \$\begingroup\$ Pedantically, this approach would be less computing power than the other approaches. \$\endgroup\$ – Kenneth K. May 22 '18 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\$ – Toby Speight Jan 23 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\$
  • \$\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 '18 at 9:14

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.