Skip to main content
replaced http://stackoverflow.com/ with https://stackoverflow.com/
Source Link

Typically in JavaScript, when calling isNaN, it is just typed:

isNaN(...);

Not as a call from the Number class:

Number.isNaN(...);

Also, by calling it the way that you did, you are being inconsistent. If you are going to call isNaN through Number, then why don't you also call parseInt through Number?


The function parseInt takes a second parameter radix which is the base of the first parameter.

According to the MDN, you should always specify the radix parameter.


Why do you call parseInt on the arguments of range? In every single point of your code where you are calling this function, you are passing numerical arguments.

Calling this functions is just a waste.


In places like these:

if (arguments.length == 0){

and

if (arguments.length == 1){

You should be using the === comparison operator rather than the == comparison operator.

  1. It is good JavaScript practice.

  2. It may be a little faster.

See this SO postthis SO post for more information.


As shown in this SO postthis SO post, a much, much simpler way to reverse a string would be to do this:

return string.split("").reverse().join("");

This could potentially be faster, too, as this is using JavaScript's built in functions/methods/features.


Your is_palindrome function confuses me a little. A palindrome is defined as a word that is spelled the exact same as it's reverse. Therefore, why aren't you using the reverse function that you already made?

Now, your is_palindrome function becomes this:

return string == reverse(string);

This is much more simple than whatever you were doing.


The naming case for JavaScript is camelCase, not snake_case. You need to change the name of these functions:

is_palindrome --> isPalindrome

and

is_lychrel --> isLychrel

And, make sure you change the name of some of your variables to camelCase too.


You are putting too much work on your isLychrel function.

Here, you check to see that if there is no recursions specified, set it to 50:

if (arguments.length == 1)
    recursions = 50;

However, this check is going to happen every single iteration (since the function is recursive).

Is is really that difficult to just pass 50 to it in the initial time you call it in main? If you did this and removed the check from the function, this could greatly improve your performance.


Typically in JavaScript, when calling isNaN, it is just typed:

isNaN(...);

Not as a call from the Number class:

Number.isNaN(...);

Also, by calling it the way that you did, you are being inconsistent. If you are going to call isNaN through Number, then why don't you also call parseInt through Number?


The function parseInt takes a second parameter radix which is the base of the first parameter.

According to the MDN, you should always specify the radix parameter.


Why do you call parseInt on the arguments of range? In every single point of your code where you are calling this function, you are passing numerical arguments.

Calling this functions is just a waste.


In places like these:

if (arguments.length == 0){

and

if (arguments.length == 1){

You should be using the === comparison operator rather than the == comparison operator.

  1. It is good JavaScript practice.

  2. It may be a little faster.

See this SO post for more information.


As shown in this SO post, a much, much simpler way to reverse a string would be to do this:

return string.split("").reverse().join("");

This could potentially be faster, too, as this is using JavaScript's built in functions/methods/features.


Your is_palindrome function confuses me a little. A palindrome is defined as a word that is spelled the exact same as it's reverse. Therefore, why aren't you using the reverse function that you already made?

Now, your is_palindrome function becomes this:

return string == reverse(string);

This is much more simple than whatever you were doing.


The naming case for JavaScript is camelCase, not snake_case. You need to change the name of these functions:

is_palindrome --> isPalindrome

and

is_lychrel --> isLychrel

And, make sure you change the name of some of your variables to camelCase too.


You are putting too much work on your isLychrel function.

Here, you check to see that if there is no recursions specified, set it to 50:

if (arguments.length == 1)
    recursions = 50;

However, this check is going to happen every single iteration (since the function is recursive).

Is is really that difficult to just pass 50 to it in the initial time you call it in main? If you did this and removed the check from the function, this could greatly improve your performance.


Typically in JavaScript, when calling isNaN, it is just typed:

isNaN(...);

Not as a call from the Number class:

Number.isNaN(...);

Also, by calling it the way that you did, you are being inconsistent. If you are going to call isNaN through Number, then why don't you also call parseInt through Number?


The function parseInt takes a second parameter radix which is the base of the first parameter.

According to the MDN, you should always specify the radix parameter.


Why do you call parseInt on the arguments of range? In every single point of your code where you are calling this function, you are passing numerical arguments.

Calling this functions is just a waste.


In places like these:

if (arguments.length == 0){

and

if (arguments.length == 1){

You should be using the === comparison operator rather than the == comparison operator.

  1. It is good JavaScript practice.

  2. It may be a little faster.

See this SO post for more information.


As shown in this SO post, a much, much simpler way to reverse a string would be to do this:

return string.split("").reverse().join("");

This could potentially be faster, too, as this is using JavaScript's built in functions/methods/features.


Your is_palindrome function confuses me a little. A palindrome is defined as a word that is spelled the exact same as it's reverse. Therefore, why aren't you using the reverse function that you already made?

Now, your is_palindrome function becomes this:

return string == reverse(string);

This is much more simple than whatever you were doing.


The naming case for JavaScript is camelCase, not snake_case. You need to change the name of these functions:

is_palindrome --> isPalindrome

and

is_lychrel --> isLychrel

And, make sure you change the name of some of your variables to camelCase too.


You are putting too much work on your isLychrel function.

Here, you check to see that if there is no recursions specified, set it to 50:

if (arguments.length == 1)
    recursions = 50;

However, this check is going to happen every single iteration (since the function is recursive).

Is is really that difficult to just pass 50 to it in the initial time you call it in main? If you did this and removed the check from the function, this could greatly improve your performance.


Source Link
SirPython
  • 13.3k
  • 3
  • 38
  • 93

Typically in JavaScript, when calling isNaN, it is just typed:

isNaN(...);

Not as a call from the Number class:

Number.isNaN(...);

Also, by calling it the way that you did, you are being inconsistent. If you are going to call isNaN through Number, then why don't you also call parseInt through Number?


The function parseInt takes a second parameter radix which is the base of the first parameter.

According to the MDN, you should always specify the radix parameter.


Why do you call parseInt on the arguments of range? In every single point of your code where you are calling this function, you are passing numerical arguments.

Calling this functions is just a waste.


In places like these:

if (arguments.length == 0){

and

if (arguments.length == 1){

You should be using the === comparison operator rather than the == comparison operator.

  1. It is good JavaScript practice.

  2. It may be a little faster.

See this SO post for more information.


As shown in this SO post, a much, much simpler way to reverse a string would be to do this:

return string.split("").reverse().join("");

This could potentially be faster, too, as this is using JavaScript's built in functions/methods/features.


Your is_palindrome function confuses me a little. A palindrome is defined as a word that is spelled the exact same as it's reverse. Therefore, why aren't you using the reverse function that you already made?

Now, your is_palindrome function becomes this:

return string == reverse(string);

This is much more simple than whatever you were doing.


The naming case for JavaScript is camelCase, not snake_case. You need to change the name of these functions:

is_palindrome --> isPalindrome

and

is_lychrel --> isLychrel

And, make sure you change the name of some of your variables to camelCase too.


You are putting too much work on your isLychrel function.

Here, you check to see that if there is no recursions specified, set it to 50:

if (arguments.length == 1)
    recursions = 50;

However, this check is going to happen every single iteration (since the function is recursive).

Is is really that difficult to just pass 50 to it in the initial time you call it in main? If you did this and removed the check from the function, this could greatly improve your performance.