6
\$\begingroup\$

I am new to Haskell and, coming from a OO/procedural programming background, I want to make sure my code follows the functional way of doing things. I built this short module to test if a given string is a palindrome (same backwards and forwards, case-sensitive) and I am simply wondering if this is the most "Haskell" way to do it. Any criticism is greatly appreciated.

module PalindromeTest (isPalindrome) where

isPalindrome :: String -> Bool

isPalindrome w
 | nChars <= 1           = nChars == 1
 | nChars == 2           = firstElem == lastElem
 | firstElem /= lastElem = False
 | firstElem == lastElem = isPalindrome (take (nChars - 2) (tail w))
 where firstElem = head w
       lastElem  = last w
       nChars    = length w
\$\endgroup\$
18
\$\begingroup\$

I would just write

isPalindrome w = w == reverse w

Short, and very easy to understand! And in this case it's also a lot more efficient, but that's another story...

\$\endgroup\$
  • 2
    \$\begingroup\$ I totally agree with this answer. One of the cool things of functional programming is that it makes it easier to reason at a higher abstraction level making the program much more compact and easy tu understand \$\endgroup\$ – mariosangiorgio Mar 25 '13 at 16:38
  • 2
    \$\begingroup\$ Could you point me towards an explanation as to how this produces the most efficient answer? As a procedural programmer, I can see where @mjgpy3 was coming from, so as I'm also trying to learn haskell atm, I'd like to really understand how the compiler performs. \$\endgroup\$ – Alex Chamberlain Jan 11 '15 at 9:25
10
\$\begingroup\$

@yatima2975 is dead right, there's a much easier way to do this as he has shown.

I'm going to write an answer though just because there's a couple things you're doing which you should be corrected of early as being the wrong approach in haskell.

 | nChars <= 1           = nChars == 1
 | nChars == 2           = firstElem == lastElem

In both these cases you're using a guard statement to check the length of an array, in haskell it's much more idiomatic to use matching to create cases for specific lengths, like so:

isPalindrome [] = False
isPalindrome [a] = True
isPalindrome [a,b] = a == b

Also:

 | firstElem == lastElem = isPalindrome (take (nChars - 2) (tail w))

Here you're doing math on the length, when all you need is the init and the tail, also this is your last case so you can simplify it using otherwise. But you don't even need to use a guard statement here because it's an and operation.

isPalindrome w = (head w == last w) && isPalindrome middle
  where middle = (init . tail) w

Learn your head/last/init/tail functions and get used to remember to use those. They work like so:

               Head=1
               |
               | __________Tail=[2,3,4,5]
               ||
              [1,2,3,4,5]
[1,2,3,4]=Init________||
                       |
                       |
                  5=Last

All of that said, the correct way to do this is the implementation detailed by yatima.

\$\endgroup\$
  • 1
    \$\begingroup\$ My favorite method of remembering head, tail, init, last: s3.amazonaws.com/lyah/listmonster.png \$\endgroup\$ – cdated Nov 8 '14 at 2:44
  • \$\begingroup\$ @cdated yeah that's what I was basically drawing above, I still picture that in my head :) \$\endgroup\$ – Jimmy Hoffa Nov 8 '14 at 2:45
  • \$\begingroup\$ Is there a reason to do (init . tail) w instead of init $ tail w? \$\endgroup\$ – cdated Nov 8 '14 at 2:46
  • 1
    \$\begingroup\$ @cdated not to my knowledge, just as a rule I favor composition over application, I suppose that may be a bit of style? Possibly just something I've inadvertently learned from reading others code. I feel like it's conventional to use composition over application where you can, if for no other reason than the precedence of $ can mix you up if you're not careful \$\endgroup\$ – Jimmy Hoffa Nov 8 '14 at 2:55

protected by Jamal Sep 28 at 22:56

Thank you for your interest in this question. Because it has attracted low-quality or spam answers that had to be removed, posting an answer now requires 10 reputation on this site (the association bonus does not count).

Would you like to answer one of these unanswered questions instead?

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