1

Not sure what i did wrong. I'm trying to compare 2 strings to see if they are Palindromes

function palindrome(str) {
  var rem = str.replace(/\s/g, "");
  var a = rem.split("");
  var b = a.reverse();

  for(var i = 0; i < rem.length; i++){
    if (b[i] == a[i]){
      return true;
    }
      return false;
  }
}

palindrome("not a doctor"); //this should show false, but it's showing true
Andrew Tan
  • 21
  • 1
  • 3

4 Answers4

2

The reverse method transposes the elements of the calling array object in place, mutating the array, and returning a reference to the array.

Source

That's why you get true. Seems your a and b variables point to the same object.

By the way, your approach seems somewhat flawed as others have pointed out bellow. So, a better version of solution to your task could be (not properly tested):

function isPalindrome(input) {
  var str = input.replace(/\s/g, "").toLowerCase();
  var left = 0, right = str.length - 1;
    while( left <= right) {
      if(str[left] !== str[right]) return false;
      left++; right--;
    }
  return true;
}

  console.log(isPalindrome("your string")); // false
  console.log(isPalindrome("Drawn onward")); //true

The idea is that you compare the corresponding symbols on both ends of your modified (without spaces and lowercased) string. If they don't match at some point, so this is not a palindrome.

curveball
  • 4,320
  • 15
  • 39
  • 49
  • 4
    Note: `.slice()` can be used to reverse a copy of the array – `var b = a.slice().reverse();` – [Reverse array in Javascript without mutating original array](https://stackoverflow.com/questions/30610523/reverse-array-in-javascript-without-mutating-original-array) – Jonathan Lonowski Apr 23 '17 at 15:23
0

Your code had two problems.

As noted by @curveball, your a and b variables are references to the same objects, as reverse modifies the original array.

Additionally, you are returning true as soon as the first element in the a array is equal to the first element of the b array. You must return false as soon as one element is different from another. But can only return true after comparing all the elements in the array.

function palindrome(str) {
  var rem = str.replace(/\s/g, "");
  var a = rem.split("");
  var b = a.slice().reverse();

  for(var i = 0; i < rem.length; i++){
    if (b[i] !== a[i]){
      return false;
    }
  }
  return true;
}

palindrome("not a doctor");

Additionally, an alternative (and typical) algorithm for checking whether a string is a palindrome would be to compare (after removing blanks) first position to last one, second position to second from the end, etc. That way it is not necessary to clone (and reverse) the data.

airos
  • 742
  • 5
  • 14
0

As others have said, your a and b points to same object, so you need to clone it first.

Also you must not return true immediately, better way is to check for inequality and return true after the whole cycle ends.

function palindrome(str) {
  var rem = str.replace(/\s/g, "");
  var a = rem.split("");
  var b = a.slice().reverse();
  console.log(a, b);

  for(var i = 0; i < rem.length; i++){
    if (b[i] != a[i]){
      return false;
    }
  }

  return true;
}

console.log(palindrome("lol 1"));
Martin Adámek
  • 16,771
  • 5
  • 45
  • 64
0

There are a few problems with your code.

Problem 1: You are using reverse method which mutate the original array (See the docs on reverse). So variables a and b would have the same value, that is the reversed string of the original array. What you can do instead is create a fresh new array and assign it to the variable b like so:

var b = [].concat(a).reverse();

Problem 2: You forgot to check ALL of the letters in the string. You are returning your function way too early. For example, for the input string 'aada', which is not a palindrome it will return true. This is because, your function exits as soon as evaluates the similarity of the first string of both arrays. To fix this you could do something like this:

function palindrome(str) {
  var rem = str.replace(/\s/g, "");
  var a = rem.split("");
  var b = [].concat(a).reverse();

  for(var i = 0; i < rem.length; i++){
    if (b[i] !== a[i]){
      return false;
    }
  }
  return true
}

You can even further optimise your function like this:

function palindrome(str) {
 const len = str.length;
 const halfLen = len/2;
 for (let i = 0; i < halfLen; i++) {
   if (str[i] !== str[len - 1 - i]) return false;
 }
 return true;
}

Palindrome strings read the same backwards and forwards, So you can make a comparison of the first and the last, second to second last etc until you reach the middle character.

Hope this helps.

Mμ.
  • 8,382
  • 3
  • 26
  • 36