I got this out of an article on interviews / code challenges. The challenge text goes like this:
A palindrome is a word, phrase, number, or other sequence of characters which reads the same backward or forward. Allowances may be made for adjustments to capital letters, punctuation, and word dividers. Examples in English include “A man, a plan, a canal, Panama!”, “Amor, Roma”, “race car”, “stack cats”, “step on no pets”, “taco cat”, “put it up”, “Was it a car or a cat I saw?” and “No ‘x’ in Nixon”.
Write the most efficient function you can that determines whether a given string is a palindrome.
Your function should accept a string as a parameter and return a boolean (true if the string is a palindrome, false if it is not).
Assume that this code will be put into a real production system and write accordingly.
Article in question: https://blog.devmastery.com/how-to-win-the-coding-interview-71ae7102d685
My Implementation:
Even though the article provides a way to solve it, my implementation is different in a few ways, so I would like your reasoning on it.
/*
* Reverses a given string of characters
*
* @param {string} string - the string to reverse
* @returns {string} - the reversed string
*
*/
const reverseString = string => string.split('').reverse().join('');
/*
* Function to test if a given string is a palindrome
*
* @param {string} string - the string to test
* @returns {boolean | string} - returns true/false based on the string, being a palindrome,
* or string if there is an error
*
*/
const isPalindrome = string => {
if (string) {
let regexReplacer = /\.|\,| |\!|\?|\'|\"|\/|\\|\-|\_|\+|\‘|\’/g;
let cleanString = string.replace(regexReplacer, '');
return (cleanString.toLowerCase() === reverseString(cleanString).toLowerCase());
}
return '[isPalindrome]: string not supplied, exiting!';
};
/*
* Basic function to test equality, poor-man's unit test
*
* @param {string} test - the string to test
* @param {string | boolean} reponse - the expected result
* @param {function} [func = isPalindrome] - the function to test, defaults to isPalindrome
*
*/
const expectToEqual = (test, reponse, func = isPalindrome) => {
if (func(test) === reponse) { console.info('Test for "' + test + '" passed!'); }
else { console.error('Test for "' + test + '" failed!'); }
};
expectToEqual('A man, a plan, a canal, Panama!', true);
expectToEqual('raceca', false);
expectToEqual('No \‘x\’ in Nixon', true);
expectToEqual('', '[isPalindrome]: string not supplied, exiting!');
Link to jsbin: https://jsbin.com/hilatev/edit?js,console
Questions:
- Regex - I'm using
/\.|\,| |\!|\?|\'|\"|\/|\\|\-|\_|\+|\‘|\’/g
to replace non word characters versus this/[^a-z0–9]/ig
which is kind of obvious. But would still like your input. - String testing - The implementation provided in the article goes though the string in a while loop and tests each character. Is this favorable vs how I just compared the strings ? Why ?
- Returns - The palindrome function returns true/false based on the string, but also returns a string if there is no string supplied. Is this favorable or I should of just thrown an error ?
- Edge cases - In hindsight I'm missing and obvious big case, after the regex replace, what if you end up with an empty string? Besides this did I miss anything else major ? Or minor ?
- Tests - I've written my own small test function vs in the article it just prints out the expected result (Ex:
console.log(isPalindrome("racecar") + " = true");
). Is this favorable? Why? - Other - Things that I may have missed ?