Palindrome Checker Feedback

Code Feedback
1

Github
Preview

I need some feedback on my javascript. While what I have is functional I wonder if there’s a better way to do it.

const textInput = document.getElementById("text-input");
const checkBtn = document.getElementById("check-btn");
let resultBox = document.getElementById("result");
let purifiedString = "";
let reversedString = "";

function showMsg() {
  if (isPalindrome()) {
    resultBox.innerText = `${textInput.value} is a palindrome.`;
  } else {
    resultBox.innerText = `${textInput.value} is not a palindrome.`;
  }
}

function purifyString(string) {
  const pattern = /[$\s\p{P}]/gu;
  purifiedString = string.replace(pattern, "");
  purifiedString = purifiedString.toLowerCase();
  console.log(purifiedString);
}
function reverseString(string) {
  let resultArr = string.split("");
  resultArr.reverse();
  reversedString = resultArr.join("");
  reversedString = reversedString.toLocaleLowerCase();
  console.log(reversedString);
}

const isPalindrome = () => (reversedString === purifiedString ? true : false);

checkBtn.addEventListener("click", () => {
  purifyString(textInput.value);
  if (purifiedString === "") {
    alert("Please Input a value");
    return;
  }
  reverseString(purifiedString);
  console.log(isPalindrome());
  showMsg();
  textInput.value = "";
});
2

Thing that’s sticking out the most is using global variables - purifiedString and reversedString. Try to modify your functions to accept arguments and return the result instead.

1 Like
3

Thank you for taking the time to review my code. Is this better?

const textInput = document.getElementById("text-input");
const checkBtn = document.getElementById("check-btn");
let resultBox = document.getElementById("result");

function showMsg() {
  if (isPalindrome(textInput.value)) {
    resultBox.innerText = `${textInput.value} is a palindrome.`;
  } else {
    resultBox.innerText = `${textInput.value} is not a palindrome.`;
  }
}

function purifyString(string) {
  const pattern = /[$\s\p{P}]/gu;
  return string.replace(pattern, "").toLowerCase();
}
function reverseString(string) {
  let result = string.split("").reverse().join("").toLocaleLowerCase();
  console.log(result);
  return result;
}

const isPalindrome = (string) => {
  return purifyString(string) === reverseString(purifyString(string))
    ? true
    : false;
};

checkBtn.addEventListener("click", () => {
  isPalindrome(textInput.value);
  if (purifyString(textInput.value) === "") {
    alert("Please Input a value");
    return;
  }
  showMsg();
  textInput.value = "";
});
4

Hi there!

Nice work. By the way you can also do these all coding logic without any function directly within checkBtn event listener call back function.

5

Hi!
Idk why but js online launchers cannot read properties of null (reading ‘addEventListener’). Anyway u can have the same result without functions on the checkBtn event listener call back function

1 Like
6

Read the conversation on the link below, hope can you find it helpful:

7

You do not need a ternary in the isPalindrome function. The expression someValue === someOtherValue already returns true or false.

1 Like
8

How do you mean? Like using using onclick dot notation or using onclick attribute on html?

9

or creating the function and the passing it as an second argument of the event listener?

10

@hasanzaib1389 said the same thing but I’m not really sure what it means. Do you mean create the function and pass the function as the second argument of event listener?

11

Thanks. I remember this being mentioned in the lesson but forgot about it while writing the code.

12

Example:

clickBtn.addeventListener("click", () => {
/* Your all DOM assignment and code logic goes here */
})
14

So I should put everything inside? is it bad habit to do it the way I did?

15

No, you did right. I gave only a suggestion, that way you can achieve the result with less code and effort. Because it is a small and simple project. For large coding project you may need to create a lot of function.

16

it this better?

const checkBtn = document.getElementById("check-btn");

checkBtn.addEventListener("click", () => {
  const textInput = document.getElementById("text-input");
  let resultBox = document.getElementById("result");
  if (textInput.value === "") {
    alert("Please Input a value");
    return;
  }
  const isPalindrome = (string) => {
    const pattern = /[$\s\p{P}]/gu;
    const purifiedString = string.trim().replace(pattern, "").toLowerCase();
    const reversedString = purifiedString
      .split("")
      .reverse()
      .join("")
      .toLowerCase();
    return purifiedString === reversedString;
  };
  function showMsg() {
    if (isPalindrome(textInput.value)) {
      resultBox.innerText = `${textInput.value} is a palindrome.`;
    } else {
      resultBox.innerText = `${textInput.value} is not a palindrome.`;
    }
  }
  showMsg();
  textInput.value = "";
});
1 Like