Easy to read palindrome checker The Next CEO of Stack OverflowPalindrome CheckerLargest Palindrome CheckerPalindrome Checker AlgorithmNumber palindrome checkerPalindrome Checker in JavaFinding palindromes in C#Python palindrome checkerTest if a string is a palindromeFinding an equilibrium index in an int arrayPython 3 palindrome checker
How to get from Geneva Airport to Metabief?
Is there a way to save my career from absolute disaster?
I believe this to be a fraud - hired, then asked to cash check and send cash as Bitcoin
Why this way of making earth uninhabitable in Interstellar?
Should I tutor a student who I know has cheated on their homework?
Reference request: Grassmannian and Plucker coordinates in type B, C, D
How to place nodes around a circle from some initial angle?
How do I align (1) and (2)?
Rotate a column
WOW air has ceased operation, can I get my tickets refunded?
What happened in Rome, when the western empire "fell"?
Would a grinding machine be a simple and workable propulsion system for an interplanetary spacecraft?
Does increasing your ability score affect your main stat?
Make solar eclipses exceedingly rare, but still have new moons
INSERT to a table from a database to other (same SQL Server) using Dynamic SQL
Writing differences on a blackboard
What steps are necessary to read a Modern SSD in Medieval Europe?
Axiom Schema vs Axiom
Find non-case sensitive string in a mixed list of elements?
Does Germany produce more waste than the US?
Can a Bladesinger Wizard use Bladesong with a Hand Crossbow?
"misplaced omit" error when >centering columns
Is wanting to ask what to write an indication that you need to change your story?
Can we say or write : "No, it'sn't"?
Easy to read palindrome checker
The Next CEO of Stack OverflowPalindrome CheckerLargest Palindrome CheckerPalindrome Checker AlgorithmNumber palindrome checkerPalindrome Checker in JavaFinding palindromes in C#Python palindrome checkerTest if a string is a palindromeFinding an equilibrium index in an int arrayPython 3 palindrome checker
$begingroup$
I made a palindrome checker that's supposed to be designed to be simple and easy to read. Please let me know what you think. I believe the time complexity is $mathcalO(n)$ but I'm not too sure about that:
Challenge: You'll need to remove all non-alphanumeric characters (punctuation, spaces and symbols) and turn everything into the same case (lower or upper case) in order to check for palindromes.
function reverseString(str)
_/g, "").toLowerCase().split(" ").join("");
var array = [];
for(var i = str.length ; i >=0; i--)
array.push(str[i])
return(array.join(""));
reverseString("My age is 0, 0 si ega ym.");
function palindrome(str) _/g, "").toLowerCase().split(" ").join("");
if(str === reverseString(str))
return true;
else
return false;
javascript algorithm programming-challenge array palindrome
$endgroup$
add a comment |
$begingroup$
I made a palindrome checker that's supposed to be designed to be simple and easy to read. Please let me know what you think. I believe the time complexity is $mathcalO(n)$ but I'm not too sure about that:
Challenge: You'll need to remove all non-alphanumeric characters (punctuation, spaces and symbols) and turn everything into the same case (lower or upper case) in order to check for palindromes.
function reverseString(str)
_/g, "").toLowerCase().split(" ").join("");
var array = [];
for(var i = str.length ; i >=0; i--)
array.push(str[i])
return(array.join(""));
reverseString("My age is 0, 0 si ega ym.");
function palindrome(str) _/g, "").toLowerCase().split(" ").join("");
if(str === reverseString(str))
return true;
else
return false;
javascript algorithm programming-challenge array palindrome
$endgroup$
$begingroup$
Are you sure this is working as intended?
$endgroup$
– Mast
5 hours ago
$begingroup$
The testreverseString("My age is 0, 0 si ega ym.");
should at least be something likeconsole.log(palindrome(reverseString("My age is 0, 0 si ega ym.")));
, and does your challenge ignore spaces? Because if they don't, your example string should return false while it doesn't. Please clarify the exact challenge.
$endgroup$
– Mast
5 hours ago
1
$begingroup$
Sorry I updated it
$endgroup$
– DreamVision2017
5 hours ago
$begingroup$
Much better, thank you.
$endgroup$
– Mast
5 hours ago
$begingroup$
Np, let me know if there's anything else that needs to be changed
$endgroup$
– DreamVision2017
5 hours ago
add a comment |
$begingroup$
I made a palindrome checker that's supposed to be designed to be simple and easy to read. Please let me know what you think. I believe the time complexity is $mathcalO(n)$ but I'm not too sure about that:
Challenge: You'll need to remove all non-alphanumeric characters (punctuation, spaces and symbols) and turn everything into the same case (lower or upper case) in order to check for palindromes.
function reverseString(str)
_/g, "").toLowerCase().split(" ").join("");
var array = [];
for(var i = str.length ; i >=0; i--)
array.push(str[i])
return(array.join(""));
reverseString("My age is 0, 0 si ega ym.");
function palindrome(str) _/g, "").toLowerCase().split(" ").join("");
if(str === reverseString(str))
return true;
else
return false;
javascript algorithm programming-challenge array palindrome
$endgroup$
I made a palindrome checker that's supposed to be designed to be simple and easy to read. Please let me know what you think. I believe the time complexity is $mathcalO(n)$ but I'm not too sure about that:
Challenge: You'll need to remove all non-alphanumeric characters (punctuation, spaces and symbols) and turn everything into the same case (lower or upper case) in order to check for palindromes.
function reverseString(str)
_/g, "").toLowerCase().split(" ").join("");
var array = [];
for(var i = str.length ; i >=0; i--)
array.push(str[i])
return(array.join(""));
reverseString("My age is 0, 0 si ega ym.");
function palindrome(str) _/g, "").toLowerCase().split(" ").join("");
if(str === reverseString(str))
return true;
else
return false;
javascript algorithm programming-challenge array palindrome
javascript algorithm programming-challenge array palindrome
edited 1 hour ago
200_success
130k17156420
130k17156420
asked 5 hours ago
DreamVision2017DreamVision2017
514
514
$begingroup$
Are you sure this is working as intended?
$endgroup$
– Mast
5 hours ago
$begingroup$
The testreverseString("My age is 0, 0 si ega ym.");
should at least be something likeconsole.log(palindrome(reverseString("My age is 0, 0 si ega ym.")));
, and does your challenge ignore spaces? Because if they don't, your example string should return false while it doesn't. Please clarify the exact challenge.
$endgroup$
– Mast
5 hours ago
1
$begingroup$
Sorry I updated it
$endgroup$
– DreamVision2017
5 hours ago
$begingroup$
Much better, thank you.
$endgroup$
– Mast
5 hours ago
$begingroup$
Np, let me know if there's anything else that needs to be changed
$endgroup$
– DreamVision2017
5 hours ago
add a comment |
$begingroup$
Are you sure this is working as intended?
$endgroup$
– Mast
5 hours ago
$begingroup$
The testreverseString("My age is 0, 0 si ega ym.");
should at least be something likeconsole.log(palindrome(reverseString("My age is 0, 0 si ega ym.")));
, and does your challenge ignore spaces? Because if they don't, your example string should return false while it doesn't. Please clarify the exact challenge.
$endgroup$
– Mast
5 hours ago
1
$begingroup$
Sorry I updated it
$endgroup$
– DreamVision2017
5 hours ago
$begingroup$
Much better, thank you.
$endgroup$
– Mast
5 hours ago
$begingroup$
Np, let me know if there's anything else that needs to be changed
$endgroup$
– DreamVision2017
5 hours ago
$begingroup$
Are you sure this is working as intended?
$endgroup$
– Mast
5 hours ago
$begingroup$
Are you sure this is working as intended?
$endgroup$
– Mast
5 hours ago
$begingroup$
The test
reverseString("My age is 0, 0 si ega ym.");
should at least be something like console.log(palindrome(reverseString("My age is 0, 0 si ega ym.")));
, and does your challenge ignore spaces? Because if they don't, your example string should return false while it doesn't. Please clarify the exact challenge.$endgroup$
– Mast
5 hours ago
$begingroup$
The test
reverseString("My age is 0, 0 si ega ym.");
should at least be something like console.log(palindrome(reverseString("My age is 0, 0 si ega ym.")));
, and does your challenge ignore spaces? Because if they don't, your example string should return false while it doesn't. Please clarify the exact challenge.$endgroup$
– Mast
5 hours ago
1
1
$begingroup$
Sorry I updated it
$endgroup$
– DreamVision2017
5 hours ago
$begingroup$
Sorry I updated it
$endgroup$
– DreamVision2017
5 hours ago
$begingroup$
Much better, thank you.
$endgroup$
– Mast
5 hours ago
$begingroup$
Much better, thank you.
$endgroup$
– Mast
5 hours ago
$begingroup$
Np, let me know if there's anything else that needs to be changed
$endgroup$
– DreamVision2017
5 hours ago
$begingroup$
Np, let me know if there's anything else that needs to be changed
$endgroup$
– DreamVision2017
5 hours ago
add a comment |
3 Answers
3
active
oldest
votes
$begingroup$
Time complexity
Your time complexity is linear but you can save a few traversals over the string and lower the constant factor as you improve readability.
Repeated code
Repeated code harms maintainability and readability. Notice that the line
str.replace(/[^ws]|_/g, "").toLowerCase().split(" ").join("");
appears in two places in the code. If you decide to change one to accept a different regex but forget to change the other one, you've introduced a potentially subtle bug into your program. Move this to its own function to avoid duplication.
Use accurate function names and use builtins
reverseString
is a confusing function: it does more than reversing a string as advertised: it also strips whitespace and punctuation, which would be very surprising if I called this function as a user of your library without knowing its internals. All functions should operate as black boxes that perform the task they claim to, nothing more or less.
The array prototype already has a reverse()
function, so there's no need to write this out by hand.
Avoid unnecessary verbosity
The code:
if(str === reverseString(str))
return true;
else
return false;
is clearer written as return str === reverseString(str);
, which says "return the logical result of comparing str
and its reversal".
Improve the regex to match your specification
Including spaces in your regex substitution to ""
is easier than .split(" ").join("")
. If you wish to remove all non-alphanumeric characters, a regex like /[^a-zd]/gi
reflects your written specification accurately (or use W
if you don't mind including underscores).
Style remarks
- JS uses K&R braces instead of Allman by convention.
- Add a blank line above
for
andif
blocks to ease vertical congestion. - Add a space around keywords and operators like
for(
and>=0
, which are clearer asfor (
and>= 0
. - No need for parentheses around a
return
value. array.push(str[i])
is missing a semicolon.- CodeReview's snippet autoformatter will automatically do most of this for you.
A rewrite
const palindrome = str =>
str = str.replace(/[^a-zd]/gi, "").toLowerCase();
return str === str.split("").reverse().join("");
;
console.log(palindrome("My age is 0, 0 si ega ym."));
$endgroup$
$begingroup$
Good points, just to make sure is the time complexity O(n) because the reverse function traverses through each element of the array?
$endgroup$
– DreamVision2017
2 hours ago
$begingroup$
Your code makes ~11 trips over then
-sized input, which is why I mention the high constant factor. If you do the replacement and lowercasing one time, you can get away with about 6 trips through the input. I count any array function, loop or===
as one trip over the input. This is a pretty minor concern relative to the other points, though, and addressing the style points accidentally improves your performance along the way.
$endgroup$
– ggorlen
2 hours ago
add a comment |
$begingroup$
Too much code.
- You can return a boolean
Note that the positions of and
if(str === reverseString(str))
return true;
else
return false;
Becomes
return str === reverseString(str);
You can remove whites spaces and commas etc with regExp
/W/g
Array has a reverse function which you can use rather than do it manually.
You should reverse the string in the function.
Strings are iterate-able so you can convert a string to an array with
[...str]
Example
function isPalindrome(str)
str = str.replace(/W/g,"").toLowerCase();
return str === [...str].reverse().join("");
$endgroup$
$begingroup$
Ah I see, btw I tried to code from scratch as much as possible to get better at problem solving/ programming. Although you are right that there are many JS methods that would make it easier to implement a solution.
$endgroup$
– DreamVision2017
2 hours ago
add a comment |
$begingroup$
The line to scrub punctuation and spaces could be simplified from:
str = str.replace(/[^ws]|_/g, "").toLowerCase().split(" ").join("");
to:
str = str.replace(/[^w]|_/g, "").toLowerCase();
Essentially, your original regex marks spaces as legal characters, which you're then going and later scrubbing out with .split(" ").join("")
. By excluding the s
in your regex, you cause the regex to match spaces in the string, which would then be replaced along with the punctuation in the str.replace method. See this regex101.
I'd also ask you to consider what it means to be a palindrome. Words like racecar
. The way you're currently doing it is by reversing the string, and then checking equality. I suggest it could be half (worst case) or O(1) (best case) the complexity if you'd think about how you could check the front and the back of the string at the same time. I won't give you the code how to do this, but I'll outline the algorithm. Consider the word Urithiru
, a faster way to check palindrome-ness would to be doing something like this: Take the first and last letters, compare them, if true, then grab the next in sequence (next from the start; next from reverse). Essentially the program would evaluate it in these steps:
u
==u
: truer
==r
: truei
==i
: truet
==h
: false
Words like Urithiru
and palindromes have the worst complexity cases for the algorithm because every letter must be checked to prove it's a palidrome. However, if you checked a work like supercalifragilisticexpialidocious
, it'd only take two iterations, and then most words in the English language (the ones that don't start and end with the same letters), would be an O(1) result. For instance, English
would fail after the first comparison.
$endgroup$
add a comment |
Your Answer
StackExchange.ifUsing("editor", function ()
return StackExchange.using("mathjaxEditing", function ()
StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix)
StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
);
);
, "mathjax-editing");
StackExchange.ifUsing("editor", function ()
StackExchange.using("externalEditor", function ()
StackExchange.using("snippets", function ()
StackExchange.snippets.init();
);
);
, "code-snippets");
StackExchange.ready(function()
var channelOptions =
tags: "".split(" "),
id: "196"
;
initTagRenderer("".split(" "), "".split(" "), channelOptions);
StackExchange.using("externalEditor", function()
// Have to fire editor after snippets, if snippets enabled
if (StackExchange.settings.snippets.snippetsEnabled)
StackExchange.using("snippets", function()
createEditor();
);
else
createEditor();
);
function createEditor()
StackExchange.prepareEditor(
heartbeatType: 'answer',
autoActivateHeartbeat: false,
convertImagesToLinks: false,
noModals: true,
showLowRepImageUploadWarning: true,
reputationToPostImages: null,
bindNavPrevention: true,
postfix: "",
imageUploader:
brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
allowUrls: true
,
onDemand: true,
discardSelector: ".discard-answer"
,immediatelyShowMarkdownHelp:true
);
);
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f216534%2feasy-to-read-palindrome-checker%23new-answer', 'question_page');
);
Post as a guest
Required, but never shown
3 Answers
3
active
oldest
votes
3 Answers
3
active
oldest
votes
active
oldest
votes
active
oldest
votes
$begingroup$
Time complexity
Your time complexity is linear but you can save a few traversals over the string and lower the constant factor as you improve readability.
Repeated code
Repeated code harms maintainability and readability. Notice that the line
str.replace(/[^ws]|_/g, "").toLowerCase().split(" ").join("");
appears in two places in the code. If you decide to change one to accept a different regex but forget to change the other one, you've introduced a potentially subtle bug into your program. Move this to its own function to avoid duplication.
Use accurate function names and use builtins
reverseString
is a confusing function: it does more than reversing a string as advertised: it also strips whitespace and punctuation, which would be very surprising if I called this function as a user of your library without knowing its internals. All functions should operate as black boxes that perform the task they claim to, nothing more or less.
The array prototype already has a reverse()
function, so there's no need to write this out by hand.
Avoid unnecessary verbosity
The code:
if(str === reverseString(str))
return true;
else
return false;
is clearer written as return str === reverseString(str);
, which says "return the logical result of comparing str
and its reversal".
Improve the regex to match your specification
Including spaces in your regex substitution to ""
is easier than .split(" ").join("")
. If you wish to remove all non-alphanumeric characters, a regex like /[^a-zd]/gi
reflects your written specification accurately (or use W
if you don't mind including underscores).
Style remarks
- JS uses K&R braces instead of Allman by convention.
- Add a blank line above
for
andif
blocks to ease vertical congestion. - Add a space around keywords and operators like
for(
and>=0
, which are clearer asfor (
and>= 0
. - No need for parentheses around a
return
value. array.push(str[i])
is missing a semicolon.- CodeReview's snippet autoformatter will automatically do most of this for you.
A rewrite
const palindrome = str =>
str = str.replace(/[^a-zd]/gi, "").toLowerCase();
return str === str.split("").reverse().join("");
;
console.log(palindrome("My age is 0, 0 si ega ym."));
$endgroup$
$begingroup$
Good points, just to make sure is the time complexity O(n) because the reverse function traverses through each element of the array?
$endgroup$
– DreamVision2017
2 hours ago
$begingroup$
Your code makes ~11 trips over then
-sized input, which is why I mention the high constant factor. If you do the replacement and lowercasing one time, you can get away with about 6 trips through the input. I count any array function, loop or===
as one trip over the input. This is a pretty minor concern relative to the other points, though, and addressing the style points accidentally improves your performance along the way.
$endgroup$
– ggorlen
2 hours ago
add a comment |
$begingroup$
Time complexity
Your time complexity is linear but you can save a few traversals over the string and lower the constant factor as you improve readability.
Repeated code
Repeated code harms maintainability and readability. Notice that the line
str.replace(/[^ws]|_/g, "").toLowerCase().split(" ").join("");
appears in two places in the code. If you decide to change one to accept a different regex but forget to change the other one, you've introduced a potentially subtle bug into your program. Move this to its own function to avoid duplication.
Use accurate function names and use builtins
reverseString
is a confusing function: it does more than reversing a string as advertised: it also strips whitespace and punctuation, which would be very surprising if I called this function as a user of your library without knowing its internals. All functions should operate as black boxes that perform the task they claim to, nothing more or less.
The array prototype already has a reverse()
function, so there's no need to write this out by hand.
Avoid unnecessary verbosity
The code:
if(str === reverseString(str))
return true;
else
return false;
is clearer written as return str === reverseString(str);
, which says "return the logical result of comparing str
and its reversal".
Improve the regex to match your specification
Including spaces in your regex substitution to ""
is easier than .split(" ").join("")
. If you wish to remove all non-alphanumeric characters, a regex like /[^a-zd]/gi
reflects your written specification accurately (or use W
if you don't mind including underscores).
Style remarks
- JS uses K&R braces instead of Allman by convention.
- Add a blank line above
for
andif
blocks to ease vertical congestion. - Add a space around keywords and operators like
for(
and>=0
, which are clearer asfor (
and>= 0
. - No need for parentheses around a
return
value. array.push(str[i])
is missing a semicolon.- CodeReview's snippet autoformatter will automatically do most of this for you.
A rewrite
const palindrome = str =>
str = str.replace(/[^a-zd]/gi, "").toLowerCase();
return str === str.split("").reverse().join("");
;
console.log(palindrome("My age is 0, 0 si ega ym."));
$endgroup$
$begingroup$
Good points, just to make sure is the time complexity O(n) because the reverse function traverses through each element of the array?
$endgroup$
– DreamVision2017
2 hours ago
$begingroup$
Your code makes ~11 trips over then
-sized input, which is why I mention the high constant factor. If you do the replacement and lowercasing one time, you can get away with about 6 trips through the input. I count any array function, loop or===
as one trip over the input. This is a pretty minor concern relative to the other points, though, and addressing the style points accidentally improves your performance along the way.
$endgroup$
– ggorlen
2 hours ago
add a comment |
$begingroup$
Time complexity
Your time complexity is linear but you can save a few traversals over the string and lower the constant factor as you improve readability.
Repeated code
Repeated code harms maintainability and readability. Notice that the line
str.replace(/[^ws]|_/g, "").toLowerCase().split(" ").join("");
appears in two places in the code. If you decide to change one to accept a different regex but forget to change the other one, you've introduced a potentially subtle bug into your program. Move this to its own function to avoid duplication.
Use accurate function names and use builtins
reverseString
is a confusing function: it does more than reversing a string as advertised: it also strips whitespace and punctuation, which would be very surprising if I called this function as a user of your library without knowing its internals. All functions should operate as black boxes that perform the task they claim to, nothing more or less.
The array prototype already has a reverse()
function, so there's no need to write this out by hand.
Avoid unnecessary verbosity
The code:
if(str === reverseString(str))
return true;
else
return false;
is clearer written as return str === reverseString(str);
, which says "return the logical result of comparing str
and its reversal".
Improve the regex to match your specification
Including spaces in your regex substitution to ""
is easier than .split(" ").join("")
. If you wish to remove all non-alphanumeric characters, a regex like /[^a-zd]/gi
reflects your written specification accurately (or use W
if you don't mind including underscores).
Style remarks
- JS uses K&R braces instead of Allman by convention.
- Add a blank line above
for
andif
blocks to ease vertical congestion. - Add a space around keywords and operators like
for(
and>=0
, which are clearer asfor (
and>= 0
. - No need for parentheses around a
return
value. array.push(str[i])
is missing a semicolon.- CodeReview's snippet autoformatter will automatically do most of this for you.
A rewrite
const palindrome = str =>
str = str.replace(/[^a-zd]/gi, "").toLowerCase();
return str === str.split("").reverse().join("");
;
console.log(palindrome("My age is 0, 0 si ega ym."));
$endgroup$
Time complexity
Your time complexity is linear but you can save a few traversals over the string and lower the constant factor as you improve readability.
Repeated code
Repeated code harms maintainability and readability. Notice that the line
str.replace(/[^ws]|_/g, "").toLowerCase().split(" ").join("");
appears in two places in the code. If you decide to change one to accept a different regex but forget to change the other one, you've introduced a potentially subtle bug into your program. Move this to its own function to avoid duplication.
Use accurate function names and use builtins
reverseString
is a confusing function: it does more than reversing a string as advertised: it also strips whitespace and punctuation, which would be very surprising if I called this function as a user of your library without knowing its internals. All functions should operate as black boxes that perform the task they claim to, nothing more or less.
The array prototype already has a reverse()
function, so there's no need to write this out by hand.
Avoid unnecessary verbosity
The code:
if(str === reverseString(str))
return true;
else
return false;
is clearer written as return str === reverseString(str);
, which says "return the logical result of comparing str
and its reversal".
Improve the regex to match your specification
Including spaces in your regex substitution to ""
is easier than .split(" ").join("")
. If you wish to remove all non-alphanumeric characters, a regex like /[^a-zd]/gi
reflects your written specification accurately (or use W
if you don't mind including underscores).
Style remarks
- JS uses K&R braces instead of Allman by convention.
- Add a blank line above
for
andif
blocks to ease vertical congestion. - Add a space around keywords and operators like
for(
and>=0
, which are clearer asfor (
and>= 0
. - No need for parentheses around a
return
value. array.push(str[i])
is missing a semicolon.- CodeReview's snippet autoformatter will automatically do most of this for you.
A rewrite
const palindrome = str =>
str = str.replace(/[^a-zd]/gi, "").toLowerCase();
return str === str.split("").reverse().join("");
;
console.log(palindrome("My age is 0, 0 si ega ym."));
const palindrome = str =>
str = str.replace(/[^a-zd]/gi, "").toLowerCase();
return str === str.split("").reverse().join("");
;
console.log(palindrome("My age is 0, 0 si ega ym."));
const palindrome = str =>
str = str.replace(/[^a-zd]/gi, "").toLowerCase();
return str === str.split("").reverse().join("");
;
console.log(palindrome("My age is 0, 0 si ega ym."));
edited 2 hours ago
answered 3 hours ago
ggorlenggorlen
423111
423111
$begingroup$
Good points, just to make sure is the time complexity O(n) because the reverse function traverses through each element of the array?
$endgroup$
– DreamVision2017
2 hours ago
$begingroup$
Your code makes ~11 trips over then
-sized input, which is why I mention the high constant factor. If you do the replacement and lowercasing one time, you can get away with about 6 trips through the input. I count any array function, loop or===
as one trip over the input. This is a pretty minor concern relative to the other points, though, and addressing the style points accidentally improves your performance along the way.
$endgroup$
– ggorlen
2 hours ago
add a comment |
$begingroup$
Good points, just to make sure is the time complexity O(n) because the reverse function traverses through each element of the array?
$endgroup$
– DreamVision2017
2 hours ago
$begingroup$
Your code makes ~11 trips over then
-sized input, which is why I mention the high constant factor. If you do the replacement and lowercasing one time, you can get away with about 6 trips through the input. I count any array function, loop or===
as one trip over the input. This is a pretty minor concern relative to the other points, though, and addressing the style points accidentally improves your performance along the way.
$endgroup$
– ggorlen
2 hours ago
$begingroup$
Good points, just to make sure is the time complexity O(n) because the reverse function traverses through each element of the array?
$endgroup$
– DreamVision2017
2 hours ago
$begingroup$
Good points, just to make sure is the time complexity O(n) because the reverse function traverses through each element of the array?
$endgroup$
– DreamVision2017
2 hours ago
$begingroup$
Your code makes ~11 trips over the
n
-sized input, which is why I mention the high constant factor. If you do the replacement and lowercasing one time, you can get away with about 6 trips through the input. I count any array function, loop or ===
as one trip over the input. This is a pretty minor concern relative to the other points, though, and addressing the style points accidentally improves your performance along the way.$endgroup$
– ggorlen
2 hours ago
$begingroup$
Your code makes ~11 trips over the
n
-sized input, which is why I mention the high constant factor. If you do the replacement and lowercasing one time, you can get away with about 6 trips through the input. I count any array function, loop or ===
as one trip over the input. This is a pretty minor concern relative to the other points, though, and addressing the style points accidentally improves your performance along the way.$endgroup$
– ggorlen
2 hours ago
add a comment |
$begingroup$
Too much code.
- You can return a boolean
Note that the positions of and
if(str === reverseString(str))
return true;
else
return false;
Becomes
return str === reverseString(str);
You can remove whites spaces and commas etc with regExp
/W/g
Array has a reverse function which you can use rather than do it manually.
You should reverse the string in the function.
Strings are iterate-able so you can convert a string to an array with
[...str]
Example
function isPalindrome(str)
str = str.replace(/W/g,"").toLowerCase();
return str === [...str].reverse().join("");
$endgroup$
$begingroup$
Ah I see, btw I tried to code from scratch as much as possible to get better at problem solving/ programming. Although you are right that there are many JS methods that would make it easier to implement a solution.
$endgroup$
– DreamVision2017
2 hours ago
add a comment |
$begingroup$
Too much code.
- You can return a boolean
Note that the positions of and
if(str === reverseString(str))
return true;
else
return false;
Becomes
return str === reverseString(str);
You can remove whites spaces and commas etc with regExp
/W/g
Array has a reverse function which you can use rather than do it manually.
You should reverse the string in the function.
Strings are iterate-able so you can convert a string to an array with
[...str]
Example
function isPalindrome(str)
str = str.replace(/W/g,"").toLowerCase();
return str === [...str].reverse().join("");
$endgroup$
$begingroup$
Ah I see, btw I tried to code from scratch as much as possible to get better at problem solving/ programming. Although you are right that there are many JS methods that would make it easier to implement a solution.
$endgroup$
– DreamVision2017
2 hours ago
add a comment |
$begingroup$
Too much code.
- You can return a boolean
Note that the positions of and
if(str === reverseString(str))
return true;
else
return false;
Becomes
return str === reverseString(str);
You can remove whites spaces and commas etc with regExp
/W/g
Array has a reverse function which you can use rather than do it manually.
You should reverse the string in the function.
Strings are iterate-able so you can convert a string to an array with
[...str]
Example
function isPalindrome(str)
str = str.replace(/W/g,"").toLowerCase();
return str === [...str].reverse().join("");
$endgroup$
Too much code.
- You can return a boolean
Note that the positions of and
if(str === reverseString(str))
return true;
else
return false;
Becomes
return str === reverseString(str);
You can remove whites spaces and commas etc with regExp
/W/g
Array has a reverse function which you can use rather than do it manually.
You should reverse the string in the function.
Strings are iterate-able so you can convert a string to an array with
[...str]
Example
function isPalindrome(str)
str = str.replace(/W/g,"").toLowerCase();
return str === [...str].reverse().join("");
answered 3 hours ago
Blindman67Blindman67
9,0911621
9,0911621
$begingroup$
Ah I see, btw I tried to code from scratch as much as possible to get better at problem solving/ programming. Although you are right that there are many JS methods that would make it easier to implement a solution.
$endgroup$
– DreamVision2017
2 hours ago
add a comment |
$begingroup$
Ah I see, btw I tried to code from scratch as much as possible to get better at problem solving/ programming. Although you are right that there are many JS methods that would make it easier to implement a solution.
$endgroup$
– DreamVision2017
2 hours ago
$begingroup$
Ah I see, btw I tried to code from scratch as much as possible to get better at problem solving/ programming. Although you are right that there are many JS methods that would make it easier to implement a solution.
$endgroup$
– DreamVision2017
2 hours ago
$begingroup$
Ah I see, btw I tried to code from scratch as much as possible to get better at problem solving/ programming. Although you are right that there are many JS methods that would make it easier to implement a solution.
$endgroup$
– DreamVision2017
2 hours ago
add a comment |
$begingroup$
The line to scrub punctuation and spaces could be simplified from:
str = str.replace(/[^ws]|_/g, "").toLowerCase().split(" ").join("");
to:
str = str.replace(/[^w]|_/g, "").toLowerCase();
Essentially, your original regex marks spaces as legal characters, which you're then going and later scrubbing out with .split(" ").join("")
. By excluding the s
in your regex, you cause the regex to match spaces in the string, which would then be replaced along with the punctuation in the str.replace method. See this regex101.
I'd also ask you to consider what it means to be a palindrome. Words like racecar
. The way you're currently doing it is by reversing the string, and then checking equality. I suggest it could be half (worst case) or O(1) (best case) the complexity if you'd think about how you could check the front and the back of the string at the same time. I won't give you the code how to do this, but I'll outline the algorithm. Consider the word Urithiru
, a faster way to check palindrome-ness would to be doing something like this: Take the first and last letters, compare them, if true, then grab the next in sequence (next from the start; next from reverse). Essentially the program would evaluate it in these steps:
u
==u
: truer
==r
: truei
==i
: truet
==h
: false
Words like Urithiru
and palindromes have the worst complexity cases for the algorithm because every letter must be checked to prove it's a palidrome. However, if you checked a work like supercalifragilisticexpialidocious
, it'd only take two iterations, and then most words in the English language (the ones that don't start and end with the same letters), would be an O(1) result. For instance, English
would fail after the first comparison.
$endgroup$
add a comment |
$begingroup$
The line to scrub punctuation and spaces could be simplified from:
str = str.replace(/[^ws]|_/g, "").toLowerCase().split(" ").join("");
to:
str = str.replace(/[^w]|_/g, "").toLowerCase();
Essentially, your original regex marks spaces as legal characters, which you're then going and later scrubbing out with .split(" ").join("")
. By excluding the s
in your regex, you cause the regex to match spaces in the string, which would then be replaced along with the punctuation in the str.replace method. See this regex101.
I'd also ask you to consider what it means to be a palindrome. Words like racecar
. The way you're currently doing it is by reversing the string, and then checking equality. I suggest it could be half (worst case) or O(1) (best case) the complexity if you'd think about how you could check the front and the back of the string at the same time. I won't give you the code how to do this, but I'll outline the algorithm. Consider the word Urithiru
, a faster way to check palindrome-ness would to be doing something like this: Take the first and last letters, compare them, if true, then grab the next in sequence (next from the start; next from reverse). Essentially the program would evaluate it in these steps:
u
==u
: truer
==r
: truei
==i
: truet
==h
: false
Words like Urithiru
and palindromes have the worst complexity cases for the algorithm because every letter must be checked to prove it's a palidrome. However, if you checked a work like supercalifragilisticexpialidocious
, it'd only take two iterations, and then most words in the English language (the ones that don't start and end with the same letters), would be an O(1) result. For instance, English
would fail after the first comparison.
$endgroup$
add a comment |
$begingroup$
The line to scrub punctuation and spaces could be simplified from:
str = str.replace(/[^ws]|_/g, "").toLowerCase().split(" ").join("");
to:
str = str.replace(/[^w]|_/g, "").toLowerCase();
Essentially, your original regex marks spaces as legal characters, which you're then going and later scrubbing out with .split(" ").join("")
. By excluding the s
in your regex, you cause the regex to match spaces in the string, which would then be replaced along with the punctuation in the str.replace method. See this regex101.
I'd also ask you to consider what it means to be a palindrome. Words like racecar
. The way you're currently doing it is by reversing the string, and then checking equality. I suggest it could be half (worst case) or O(1) (best case) the complexity if you'd think about how you could check the front and the back of the string at the same time. I won't give you the code how to do this, but I'll outline the algorithm. Consider the word Urithiru
, a faster way to check palindrome-ness would to be doing something like this: Take the first and last letters, compare them, if true, then grab the next in sequence (next from the start; next from reverse). Essentially the program would evaluate it in these steps:
u
==u
: truer
==r
: truei
==i
: truet
==h
: false
Words like Urithiru
and palindromes have the worst complexity cases for the algorithm because every letter must be checked to prove it's a palidrome. However, if you checked a work like supercalifragilisticexpialidocious
, it'd only take two iterations, and then most words in the English language (the ones that don't start and end with the same letters), would be an O(1) result. For instance, English
would fail after the first comparison.
$endgroup$
The line to scrub punctuation and spaces could be simplified from:
str = str.replace(/[^ws]|_/g, "").toLowerCase().split(" ").join("");
to:
str = str.replace(/[^w]|_/g, "").toLowerCase();
Essentially, your original regex marks spaces as legal characters, which you're then going and later scrubbing out with .split(" ").join("")
. By excluding the s
in your regex, you cause the regex to match spaces in the string, which would then be replaced along with the punctuation in the str.replace method. See this regex101.
I'd also ask you to consider what it means to be a palindrome. Words like racecar
. The way you're currently doing it is by reversing the string, and then checking equality. I suggest it could be half (worst case) or O(1) (best case) the complexity if you'd think about how you could check the front and the back of the string at the same time. I won't give you the code how to do this, but I'll outline the algorithm. Consider the word Urithiru
, a faster way to check palindrome-ness would to be doing something like this: Take the first and last letters, compare them, if true, then grab the next in sequence (next from the start; next from reverse). Essentially the program would evaluate it in these steps:
u
==u
: truer
==r
: truei
==i
: truet
==h
: false
Words like Urithiru
and palindromes have the worst complexity cases for the algorithm because every letter must be checked to prove it's a palidrome. However, if you checked a work like supercalifragilisticexpialidocious
, it'd only take two iterations, and then most words in the English language (the ones that don't start and end with the same letters), would be an O(1) result. For instance, English
would fail after the first comparison.
answered 1 hour ago
user138741user138741
1534
1534
add a comment |
add a comment |
Thanks for contributing an answer to Code Review Stack Exchange!
- Please be sure to answer the question. Provide details and share your research!
But avoid …
- Asking for help, clarification, or responding to other answers.
- Making statements based on opinion; back them up with references or personal experience.
Use MathJax to format equations. MathJax reference.
To learn more, see our tips on writing great answers.
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f216534%2feasy-to-read-palindrome-checker%23new-answer', 'question_page');
);
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
$begingroup$
Are you sure this is working as intended?
$endgroup$
– Mast
5 hours ago
$begingroup$
The test
reverseString("My age is 0, 0 si ega ym.");
should at least be something likeconsole.log(palindrome(reverseString("My age is 0, 0 si ega ym.")));
, and does your challenge ignore spaces? Because if they don't, your example string should return false while it doesn't. Please clarify the exact challenge.$endgroup$
– Mast
5 hours ago
1
$begingroup$
Sorry I updated it
$endgroup$
– DreamVision2017
5 hours ago
$begingroup$
Much better, thank you.
$endgroup$
– Mast
5 hours ago
$begingroup$
Np, let me know if there's anything else that needs to be changed
$endgroup$
– DreamVision2017
5 hours ago