Check for palindromes/what am i doing wrong?

function palindrome(str) {
  // Good luck!
  var newStr = str.replace(/[^A-Z0-9]/ig, "")
    .toLowerCase()
    .split("");
  var strRe = newStr.reverse();
  
  for(var i = 0; i < newStr.length; i++){
    for(var j = 0; j < strRe.length; j++){
        if(strRe[j] === newStr[i]){
            return true;
        }else 
            return false;
    }
  }

}

palindrome(“eye”);

I’m still playing about with it but I can see after putting in a few console logs var strRe = newStr.reverse(); is also reversing newStr so both variables end up as the exact same string.

   var strRe = newStr.split('').reverse().join('');

seems to fix the problem with the original string reversing as well, not sure why but perhaps someone else will have a better explanation for that. However I will read more into that as it’s fairly interesting behavior :smiley: I think perhaps it had something to do with where you split the string? Though I may be wrong.

I’ve got them all to pass accept for palindrome("almostomla"); so will play a little more ! Almost there :smiley:

Edit:

@ddamaja okay I think it has something to do with the nested loop. To be honest you don’t really even need loops to complete this. A simple if else statement will suffice after splitting and reversing the strings :slight_smile: like :

    return true;
  }
  else{
    return false;
  }``` 

Im lazy so I try to use as less code as I can. As to why your nested for loop doesn't work:

@ddamaja "remember that j starts again at 0 every time i advances... the two loops don't run in parallel."

The upper half is fine, apart from the use of Array.prototype.reverse, as already pointed out by @fattie. Your for loop does not work as intended, though:

// for every character in newStr...
  for(var i = 0; i &lt; newStr.length; i++){ 
  // ... go over every character in strRe ...
    for(var j = 0; j &lt; strRe.length; j++){
    // ... but only compare the first pair of 
    // characters, then immediately return
    //  whether they are equal or not.
        if(strRe[j] === newStr[i]){
            return true;
        }else 
            return false;
    }
  }

// in short:
return strRe[0] === newStr[0];

So your function returns whether the first character of a given string matches its last character. You should remove the inner for-loop, because an inner loop always means a 1 to many relationship, compare 1 character with many others. To check for palindromes, you want to compare 1 character with another: first with last, second with second to last etc. The second problem with your loop is that you always return after comparing the first pair of characters, either true or false. However, you only want to return in two cases:

  1. After determining that the string and the reversed string are identical, which makes it a palindrome.

  2. You find a pair of characters that are different from each other, thus the string can’t be a palindrome.

I hope that makes sense :slight_smile:

2 Likes