Skip to content

Update factorialRecursive.js by checking the factorial of negative numbers at run time#1009

Closed
njiddasalifu wants to merge 0 commit into
trekhleb:masterfrom
njiddasalifu:master
Closed

Update factorialRecursive.js by checking the factorial of negative numbers at run time#1009
njiddasalifu wants to merge 0 commit into
trekhleb:masterfrom
njiddasalifu:master

Conversation

@njiddasalifu

Copy link
Copy Markdown

added a clear check for the factorial eveluation of negative numbers to avoid wrong answer at runtime.

@lazarljubenovic

Copy link
Copy Markdown

It's bad practice to return a string or a number and then inspect it. You should throw an error for invalid input instead.

@lazarljubenovic

Copy link
Copy Markdown

There's no such thing as "documen". Also, writing to the document is probably even worse than returning a string. You should throw an error instead.

@lazarljubenovic

Copy link
Copy Markdown

You're throwing a string instead of an error now.

@njiddasalifu

njiddasalifu commented Mar 19, 2023 via email

Copy link
Copy Markdown
Author

@olgunkaya olgunkaya left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you really need that last number > 1 control ? You already guaranteed there the number is bigger than 1

@njiddasalifu

Copy link
Copy Markdown
Author

@olgunkaya ooh i see I will remove that. Actually the fisrt lines took care of that already.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants