4

I want to build a palindrome checker in javascript. All non-letter characters should be removed, so that a phrase like "A man, a plan, a canal. Panama" can also be a palindrome.

function reverse(str) {
  return str.split("").reverse().join("");
}


function palindrome(str) {
  str = str.replace(/[^a-zA-Z]+/,"").toLowerCase();
  if(str == reverse(str)) {
    return true;
  }
  else {
    return false;
  }
}

Now, where is the mistake in the above lines?

The code works on some examples. But for instance "A man, a plan, a canal. Panama" and "never odd or even" return false, meaning somewhere has to be a mistake.

k_rad
  • 59
  • 6
  • 3
    Can you provide some examples: what does work? What doesn't work? Does "abba" return `true` or not? .. Give as much information as possible :) – Jordumus Sep 04 '15 at 10:01
  • 2
    "where is the mistake?" does it not work? any errors? – atmd Sep 04 '15 at 10:01
  • 2
    From the examples I've run it works. – Andy Sep 04 '15 at 10:02
  • 2
    `String.replace()` replaces only one match unless you specify otherwise. – JJJ Sep 04 '15 at 10:04
  • [test-fiddle](http://jsfiddle.net/memnekky/) seems to be working here as well. – Jordumus Sep 04 '15 at 10:05
  • 1
    That said, learning some basic debugging skills wouldn't hurt; e.g. `console.log(str)` right after the replacement would have immediately told you where the problem is. – JJJ Sep 04 '15 at 10:05
  • Should be `str = str.replace(/[^a-z]+/ig, '').toLowerCase();` – raina77ow Sep 04 '15 at 10:05
  • 4
    To people who say it works: try it with the "A man, a plan..." test string. – JJJ Sep 04 '15 at 10:05
  • 6
    Avoid writing `if (a == b) return true else return false` when you can write `return a == b;` – Eloims Sep 04 '15 at 10:08
  • Did you debug your program? For example, put a breakpoint at the line `if (str == reverse(str))`, and examine `str` and `reverse(str)`? –  Sep 04 '15 at 10:13

5 Answers5

5

You need to provide the global match flag to your regex:

/[^a-zA-Z]+/g
            ^
sp00m
  • 47,968
  • 31
  • 142
  • 252
1

This is a common misconception. The replace() method does not replace all instances of what you want to replace in a string. It simply replaces the first instance and stops. If you refactor your regEx like this: 

function reverse(str) {
  return str.split("").reverse().join("");
}


function palindrome(str) {
    var find = "[^a-zA-Z]";
    var regEx = new RegExp(find, 'g');
  str = str.replace(regEx,"").toLowerCase();
  if(str == reverse(str)) {
    return true;
  }
  else {
    return false;
  }
}

That will work.

James Walters
  • 26
  • 3
1

From the example given, it seems to me that the code doesn't work for spaces in between the letters. (There may be other scenarios as well)

I have changed this line :

str = str.replace(/[^a-zA-Z]+/,"").toLowerCase();

To this :

str = str.toLowerCase().replace(/[^a-z]/g,"");
Soubhik Mondal
  • 2,666
  • 1
  • 13
  • 19
1

change this line:

str = str.replace(/[^a-zA-Z]+/,"").toLowerCase();

to this:

str = str.toLowerCase().replace(/[^a-z0123456789]+/g,"");
Burak Keles
  • 21
  • 4
-1

This regex should work for your code.

/[^1-9a-zA-Z]+/g

leena jethva
  • 1