Palindrome Checker - issue with for loop

Question

I'm learning javaScript and writing a palindrome checker to practice. It's not working and by using print statements I've narrowed this down to an issue with my for loop. I can't see why it wouldn't work though; could anyone possibly shed a light?

function palindrome(str) {
  var newString = str.replace(/[^a-z0-9]/g, '').toLowerCase();
  console.log(newString)
  var forwardsArray = newString.split("");
  console.log(forwardsArray)
  var backwardsArray = forwardsArray.reverse();
  console.log(backwardsArray)
  for (var i = 0; i < backwardsArray.length; i++) {
    for (var j = 0; j < forwardsArray.length; j++) {
      console.log(backwardsArray[i])
      console.log(forwardsArray[i])
      if (forwardsArray[j] !== backwardsArray[i]) {
        return false;
      }
    }
    return true;
  }
}

Your code appears to be checking every item of the array against every item of the other array. So it would only work if your input is like `aaaaaaa`. Just do `function palindrome(str) {return str === str.split("").reverse().join("");}` — Niet the Dark Absol, Feb 25 '19 at 19:46
Possible duplicate of [Palindrome check in Javascript](https://stackoverflow.com/questions/14813369/palindrome-check-in-javascript) — Igor, Feb 25 '19 at 19:48
That is a lot of code checking for a palindrome lol! But you might want to search the forum first before posting a question. — Michelangelo, Feb 25 '19 at 19:51
@NiettheDarkAbsol the extra "=" in your comment threw me off. :D.. *edit as khauri McClain points below, i've learnt something new. — schar, Feb 25 '19 at 20:06
@schar It's not an extra `=` it's a [strict equality comparison](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Equality_comparisons_and_sameness) — Khauri, Feb 25 '19 at 20:07

score 1 · Answer 1 · answered Feb 25 '19 at 20:05

I agree with the other commenters that there are better ways to find palindromes, but I think this is a good learning opportunity because there is a common beginner trap in your code that you might benefit from knowing about.

The biggest problem is course the fact that your for loop checks if every item of your backwardsArray is equal to each and every item of your forwardsArray, and unless your word contains only all similar characters, this won't be true.

In reality you just want to check if the character in your forward array is equivalent to the character in your backward array at the same index, so you only need one for loop.

But a not so obvious problem is that Array.reverse reverses the array "in place". This means that forwardsArray is modified when you call reverse instead of returning a new copy, and thus backwardsArray and forwardsArray reference the exact same array.

You should clone the array here. A popular method is just to use Array.slice

function palindrome(str) {

  var newString = str.replace(/[^a-z0-9]/g, '').toLowerCase();

  var forwardsArray = newString.split("");
  // Copy the array
  var backwardsArray = forwardsArray.slice().reverse();
  
  for (var i = 0; i < backwardsArray.length; i++) {
      var backwardChar = backwardsArray[i]
      var forewardChar = forwardsArray[i]
      if(backwardChar !== forewardChar){
        return false
      }
  }
  return true;
}

console.log(palindrome("racecar")) // true

console.log(palindrome("ralecar")) // false

FZs · Answer 2 · 2021-03-09T09:14:36.303

I made some changes to make it work:

Removed second for loop (it's unnecessary)
Used array[i] !== array[array.length-i-1] condition instead of reversing the array
Looping only to the half of the array (if the first half matches, the second half will too)

Here is the snippet:

function palindrome(str) {
  var newString = str.replace(/[^a-z0-9]/g, '').toLowerCase();
  var array = newString.split("");
  for (var i = 0; i < array.length/2; i++) {
    if (array[i] !== array[array.length-i-1]) {
      return false;
    }
  }
  return true;
}

console.log(palindrome('abcba')) //true
console.log(palindrome('abcde')) //false

Daniel · Answer 3 · 2019-10-14T06:53:22.130

By now of course you have figured out you were overthinking it.

function palindrome(str) {
  const newString = str.replace(/[^a-z0-9]/g, '').toLowerCase();
  const reversed = newString.split('').reverse().join('');

  return newString === reversed;
}

palindrome("abba"); // true
palindrome("abcd"); // false

So I left the newString logic, but after doing that, at the end of the day, you are going to take the same newString and split it, reverse it and join it and then compare it with its original string, in your case, newString.

So you do a direct comparison between reversed and your string and if they are equal then its a palindrome, if not, then its not a palindrome.

Palindrome Checker - issue with for loop

3 Answers3