Check for Palindromes - pls review my clunky code

JavaScript
1

Hi - I know my code is really clunky (even compared to the “intermediate” solution :cry: ) but it’s working and that is a victory for me for now. However, it is failing ONE of the testcases, i.e. palindrome(“almostomla”) should return false

I cannot tell why. Can someone take a look at my code? Thank you in advance!

Your code so far

  // remove non-alpha-numeric characters
  str = str.replace(/[^0-9a-z]/gi, '');
  str = str.replace(/\s/g, ''); 
  
  //make everything lowercase
  str = str.toLowerCase();
  
  
  //build arrays of str - going forwards and backwards
  var fwdarray=[];
  fwdarray = str.split('');
  
  var bkwdarray=[];
  bkwdarray = str.split('');
  bkwdarray.reverse();
    

  
  //compare the 2 arrays to see if they are the same
    for (i=0; i < fwdarray.length; i++){
//         console.log(fwdarray);
//         console.log(bkwdarray);      
      if (fwdarray[i]===bkwdarray[i]){
        return true;
      }else 
        return false;
    }
  }

palindrome("eye");

Your browser information:

Your Browser User Agent is: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/64.0.3282.186 Safari/537.36.

Link to the challenge:
https://www.freecodecamp.org/challenges/check-for-palindromes

2

Inside the for loop:
a matches a then you return true.

You don’t check all the characters.

2 Likes
4

Ugh. Well that’s pretty disheartening. (Is there any way to give feedback on that so they can make the test-cases more robust?)

If I understand correctly, I can’t return anything in the IF statement b/c it will exit the loop at the first positive result.

To fix:
I tried setting a variable to “on/off” and then checking if that var was on or off after the loop but that resulted in the same problem. Or a similar one where the variable could get overwritten if the last letter of each array matched but none of the other letters matched, giving a false positive.

I even tried a WHILE loop but I didn’t know what to put in the loop itself, besides adding 1 to counter…?

I think I’m at an impasse at this point. (Google search for comparing 2 arrays turned up some more advanced (to my eyes) javascript solutions).

Send help .

6

Thank you @camperextraordinaire! It finally worked. Since the test-cases are not robust enough to have caught my bad code before, let me know if you see any problems with latest code (besides being very clunky still):

function palindrome(str) {
  // remove non-alpha-numeric characters
  str = str.replace(/[^0-9a-z]/gi, '');
  str = str.replace(/\s/g, ''); 
  
  //make everything lowercase
  str = str.toLowerCase();
  
  
  //build arrays of str - going forwards and backwards
  var fwdarray=[];
  fwdarray = str.split('');
  
  var bkwdarray=[];
  bkwdarray = str.split('');
  bkwdarray.reverse();
    

  
  //compare the 2 arrays to see if they are the same
    for (i=0; i < fwdarray.length-1; i++){
        console.log(fwdarray);
        console.log(bkwdarray);      
      if (fwdarray[i] != bkwdarray[i]){
        return false;
      }
    }
  return true;
  }

palindrome("eye");
8

Hi there,

Can I try to help you improve your code a little bit?

function palindrome(str) {
  // remove non-alpha-numeric characters
  str = str.replace(/[^0-9a-z]/gi, '');
  str = str.replace(/\s/g, '');

In the first regular expression you’re replacing everything from the str variable, except for alphanumeric values (spaces, dots, underscores, etc). That’s because of the ^ symbol means something like everything but…. So, the second assignment is redundant as it will never do anything.

Awesome site for RegEx study.

  //make everything lowercase
  str = str.toLowerCase();
  
  
  //build arrays of str - going forwards and backwards
  var fwdarray=[];
  fwdarray = str.split('');
  
  var bkwdarray=[];
  bkwdarray = str.split('');
  bkwdarray.reverse();

Just like randelldawson mentioned, you can make this 5 lines a little shorter, but it works anyway :slight_smile:

  //compare the 2 arrays to see if they are the same
    for (i=0; i < fwdarray.length-1; i++){
        console.log(fwdarray);
        console.log(bkwdarray);      
      if (fwdarray[i] != bkwdarray[i]){
        return false;
      }
    }
  return true;
  }

palindrome("eye");

On this part I think you wouldn’t need a loop and I think if you see the code in a week you will spend some time figuring out what it does.

Might be easier using some functions coming with the language. Array.prototype.join(), for example.

My approach would be something like this:

if( str != str.split('').reverse().join('') ) return false;

You compare your string (without splitting it in your previous code) against your string, but this time splitted, reversed and glued together again as a string. If it’s false, returns false. Else, triggers the last expression in your function, which returns true.

Hope it helped.

Happy coding!

1 Like