Mryntu

I made a palindrome checker that's supposed to be designed to be simple and easy to read. Please let me know what you think. I believe the time complexity is O(n) but I'm not too sure about that:

Challenge: You'll need to remove all non-alphanumeric characters (punctuation, spaces and symbols) and turn everything into the same case (lower or upper case) in order to check for palindromes.

function reverseString(str)
_/g, "").toLowerCase().split(" ").join("");
 var array = [];
 for(var i = str.length ; i >=0; i--)
 
 array.push(str[i])
 
 return(array.join(""));



reverseString("My age is 0, 0 si ega ym.");

function palindrome(str) _/g, "").toLowerCase().split(" ").join("");
 if(str === reverseString(str))
 
 return true; 
 
 else
 
 return false;
 

Too much code.

• You can return a boolean

Note that the positions of and

 if(str === reverseString(str)) 
 return true; 
 else 
 return false;
 

Becomes

 return str === reverseString(str);

• You can remove whites spaces and commas etc with regExp /W/g




• Array has a reverse function which you can use rather than do it manually.




• You should reverse the string in the function.




• Strings are iterate-able so you can convert a string to an array with [...str]

Example

function isPalindrome(str) 
 str = str.replace(/W/g,"").toLowerCase();
 return str === [...str].reverse().join("");

Time complexity

Your time complexity is linear, although you can save a few traversals over the string and lower the constant factor.

Repeated code

One rule of thumb in a code review is to look for repeated code, which harms maintainability and readability. Notice that the line

str.replace(/[^ws]|_/g, "").toLowerCase().split(" ").join("");

appears in two places in the code. If you decide to change one to accept a different regex but forget to change the other one, you've introduced a potentially subtle bug into your program. Move this to its own function to avoid duplication.

Use accurate function names; use builtins

reverseString is a confusing function: it does more than reversing a string as advertised: it also strips whitespace and punctuation, which would be very surprising if I called this function as a user of your library without knowing its internals. All functions should operate as black boxes that perform one task advertised in their name.

The array prototype already has a reverse() function, so there's no need to write this out by hand.

Avoid unnecessary verbosity

The code:

 if(str === reverseString(str))
 
 return true; 
 
 else
 
 return false;
 

is clearer written as return str === reverseString(str);, which says "return the logical result of comparing str and its reversal".

Improve the regex to match your specification

The .split(" ").join("") operation is the same as simply including spaces in your regex substitution from " " to "". If you wish to remove all non-alphanumeric characters, a regex like /[^a-zd]/gi reflects your written specification accurately (or use W if you don't mind including underscores).

Style remarks

• JS uses K&R braces instead of Allman by convention.


• Add a blank line above for and if blocks to ease vertical congestion.


• Add a space around keywords and operators like for( and >=0, which are clearer as for ( and >= 0.


• No need for parentheses around a return value.


• 
  array.push(str[i]) is missing a semicolon.

A rewrite

const palindrome = str => 
 str = str.replace(/[^a-zd]/gi, "").toLowerCase();
 return str === str.split("").reverse().join("");
;

console.log(palindrome("My age is 0, 0 si ega ym."));