2

I'm new to Javascript and wrote the code below to determine if a string is a palindrome. I'm curious as to what is the most efficient way to accomplish the same task.

var isPalindrome = function (string) {
    var leftString = [];
    var rightString = [];

    // Remove spaces in the string and convert to an array
    var strArray = string.split(" ").join("").split("");    
    var strLength = strArray.length;

    // Determine if the string is even or odd in length, then assign left and right strings accordingly
    if (strLength % 2 !== 0) {
        leftString = strArray.slice(0, (Math.round(strLength / 2) - 1));
        rightString = strArray.slice(Math.round(strLength / 2), strLength);
    } else {
        leftString = strArray.slice(0, (strLength / 2));
        rightString = strArray.slice((strLength / 2, strLength))
    }

    if (leftString.join("") === rightString.reverse().join("")) {
        alert(string + " is a palindrome.");
    } else {
        alert(string + " is not a palindrome.")
    }

}


isPalindrome("nurses run");

6 Answers6

3
function isPalindrome( s )
{
   var i = 0, j = s.length-1;
   while( i < j )
       if( s[i++].toLowerCase() != s[j--].toLowerCase() ) return false;
   return true;
}
Matt
  • 20,108
  • 1
  • 57
  • 70
3

It's not clear if you're talking about efficiency in terms of code length, or amount of computation, but this should be fairly good in both regards. And it takes into account non-alpha characters beside spaces as well as capitalization:

function isPalindrome(str) {
   var i, len;

   str = str.toLowerCase().replace(/[^a-z]/g, '');
   len = str.length;

   for(i = 0; i < len / 2; i += 1) {
      if(str.charCodeAt(i) != str.charCodeAt(len - i - 1)) {
         return false;
      }
   }

   return true;
}

A much shorter approach (though perhaps more computation intensive):

function isPalindrome(str) {
   str = str.toLowerCase().replace(/[^a-z]/g, '');

   return str == str.split("").reverse().join("");
}

And if you really want that alert stuff, I'd suggest putting it in a separate function:

function isPalindromeAlert(str) {
  alert(str + "is " + (isPalindrome(str) ? "" : "not ") + "a palindrome.");
}
JLRishe
  • 99,490
  • 19
  • 131
  • 169
1

I think this one is lot simpler:

var isPalindrome = function (string) {
    if (string == string.split('').reverse().join('')) {
        alert(string + ' is palindrome.');
    }
    else {
        alert(string + ' is not palindrome.');
    }
}

See more: Palindrome check in Javascript

Community
  • 1
  • 1
aksu
  • 5,221
  • 5
  • 24
  • 39
  • 1
    I don't think it is efficient to split, reverse, and join. – Trenin Feb 03 '14 at 20:04
  • How does this even work? I don't understand how the splitting, reversing, and joining are working here. Unless you are making assumptions about the input that are not obvious... – Trenin Feb 03 '14 at 20:07
  • This also will always return true, the if statement never compares the string to anything, it just checks that you are able to reverse the string – Tom Feb 03 '14 at 20:08
  • Shouldn't it be `string === string.split('').reverse().join('')`? – Andy Feb 03 '14 at 20:08
  • @asku What is the point of splitting and joining? Cant you just reverse it and compare? – Trenin Feb 03 '14 at 20:09
  • Also, this isn't efficient because the split, join, and reverse operations all take at least `n` operations, where `n` is the length of the string. The two answers below all exit early as soon as they detect it is not a palindrome, so they will be more efficient. – Trenin Feb 03 '14 at 20:11
  • I think `if (string == string.reverse())` is simpler and more efficient than this answer, but not as efficient as the other answers. – Trenin Feb 03 '14 at 20:12
  • @Trenin, i can't. I must convert the string to array before `.reverse`. It can only used for arrays. – aksu Feb 03 '14 at 20:12
  • @asku - Fair enough. Still not very efficient because you are doing lots of operations on the string, then array. Better to loop through the characters of the string. – Trenin Feb 03 '14 at 20:13
1
var str = "abcba";
var len = str.Lenght;
var index = 0;

while(index <= len/2 && str[index] == str[len - index - 1]) index++;

if(index == len/2) {
    alert(string + " is a palindrome.");
}
else {
   alert(string + " is not a palindrome.");
}

You made a few unnecesary operations.

emcas88
  • 881
  • 6
  • 15
0

To be efficient, you should avoid unnecessary computations. Ask yourself:

  • do you need to remove spaces?
  • do you need to convert to an array?
  • do you need to allocate new objects for the left and right strings?
  • do you need to reverse the string?

The checking can be done in a very simple loop:

var len=string.length;
for (int i=0; i<(len/2); i++) {
  if (string[i] != string[len-i-1]) {
    alert(string + " is not a palindrome.");
    return;
  }
}
alert(string + " is a palindrome.");

To ignore spaces and non-alpha numeric characters, it can be re-written as follows:

function isAlphaNumeric( chr ) {
  return ( ((c >= 'a') && (c <= 'z')) ||
           ((c >= 'A') && (c <= 'Z')) ||
           ((c >= '0') && (c <= '9')) );
}

// Note - template taken from @Matt's answer!
function isPalindrome( string ) {
  var i = 0, j = s.length-1;
  while( i < j ) {
    if (isAlphaNumeric(string[i])) {
      if (isAlphaNumeric(string[j])) {
        if ( string[i++].toLowerCase() != string[j--].toLowerCase() ) 
          return false;
      } else {
        j--;
      }
    } else {
      i++;
      if (!isAlphaNumeric(string[j])) j--;
    }
  }
  return true;
}
Trenin
  • 2,041
  • 1
  • 14
  • 20
  • Typically, a palindrome check should ignore spaces and other non-alpha characters. – JLRishe Feb 03 '14 at 20:17
  • @JLRishe Perhaps it is more efficient to skip over those characters inside the loop instead of creating entire intermediate objects... For example, if the string is `a--------b`, then a new string `ab` is created after processing all `10` characters in the original string. Only then is the check for palindromicity (is that a word?) done. This and some other answers would see on the first check that `'a'!='b'`, so would return immediately. – Trenin Feb 03 '14 at 20:20
  • Ok, but I hope you enjoy coding that (and making sure it doesn't have any bugs). Typically, readable code is preferable to overwrought code with micro-optimizations. – JLRishe Feb 03 '14 at 20:26
  • @JLRishe OP asked for the most efficient way, not the most readable. And I do enjoy coding that! – Trenin Feb 04 '14 at 12:53
0

unreadable + unmaintainable + terse + efficient + recursive + non-branching

function isPalindrome(s,i) {
 return (i=i||0)<0|| i>=s.length/2|| s[i]==s[s.length-1-i]&& isPalindrome(s,++i);
}

Fiddle: http://jsfiddle.net/namcx0yf/

King Friday
  • 25,132
  • 12
  • 90
  • 84