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');
5 Answers 5
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.)
-
\$\begingroup\$ Smart :) I like it! \$\endgroup\$Matadeleo– Matadeleo2018年05月21日 22:12:59 +00:00Commented May 21, 2018 at 22:12
-
\$\begingroup\$ And what is the reason of the divorce? \$\endgroup\$Billal BEGUERADJ– Billal BEGUERADJ2018年05月22日 10:53:01 +00:00Commented 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– Tonny2018年05月22日 11:26:35 +00:00Commented 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– Doug2018年05月22日 12:58:09 +00:00Commented 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\$Scott Sauyet– Scott Sauyet2018年05月22日 14:13:24 +00:00Commented May 22, 2018 at 14:13
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
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
-
\$\begingroup\$ Thanks, no reason why the
$secondHalf
should be repeated, and that return is much cleaner \$\endgroup\$Matadeleo– Matadeleo2018年05月21日 18:35:40 +00:00Commented May 21, 2018 at 18:35
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');
-
1\$\begingroup\$ Pedantically, this approach would be less computing power than the other approaches. \$\endgroup\$Kenneth K.– Kenneth K.2018年05月22日 13:17:32 +00:00Commented 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\$Toby Speight– Toby Speight2019年01月23日 17:23:13 +00:00Commented Jan 23, 2019 at 17:23
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 'é_é'?
-
\$\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– Matadeleo2018年05月23日 09:14:38 +00:00Commented May 23, 2018 at 9:14
isPalindrome
function \$\endgroup\$