CODEWITHSUNDEEP
javascript
Pineapple
Pineapple

Reputation: 33

Palindrome Checker - issue with for loop

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;
  }
}

Upvotes: 0

Views: 117

Answers (3)

FZs
FZs

Reputation: 18619

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

Upvotes: 1

Daniel
Daniel

Reputation: 15453

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.

Upvotes: 1

Khauri
Khauri

Reputation: 3863

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

Upvotes: 1

Related Questions