Why is factorial snippet handling negative numbers? I do not think factorial for negative numbers is defined.
Snippet from factorial.md :
const factorial = n => n <= 1 ? 1 : n * factorial(n - 1);
the code returns 1 for any value lower than 1 and the recursion ends, which actually mean factorial(0) returns 1, which is wrong as it should return 0.
negative numbers are an invalid input and none of these example functions really have any guards or validation, so that probably isn't 'wrong' however !0 === 1 is mathematically incorrect
The factorial of 0 is actually 1. I've been taught this multiple times and a quick google search says so. So technically factorial(0) -> 1 is correct. Safeguards for negative numbers are not in place, but I expect we can easily PR that to fix the problem.
Similar safeguards can be added in multiple other examples. For example, x modulo 0 is not defined in pratice (snippet does not handle this!)
this will be helpful @monikadhok @Chalarangelo
let factorial = n => n < 0 ?
(() => { throw new TypeError('Negative numbers are not allowed!') })() : n <= 1 ?
1 : n * factorial(n - 1);
We are currently discussing, under #172, the need for error handling. In general, we're probably going to go with no error handling except if it's necessary. In the factorial snippet, I think it isn't, but I am a little opinionated on that. More opinions?
Factorials of negative numbers are dependent on if it's even or odd, and by definition are -Infinity or Infinity because there is no way for the construct( algorithm ) to stop( it is defined stopped at special case 0/-0 )
Ideally it should be
const factorial = n => n < 0 ? n % 2 == 0 ? Infinity : -Infinity : n == 0 ? 1 : n * factorial(n-1)
@skatcat31 That sounds like a good way to handle this, let's get it fixed then.
@skatcat31 It should be const factorial = n => n < 0 ? Math.abs(n) % 2 == 0 ? Infinity : -Infinity : n == 0 ? 1 : n * factorial(n-1)
Javascript gives -8%2 == -0
@kriadmin -0 === 0 === true, lets us avoid a function call
Where our snippet fix fail is in handling decimals since it is supposed to clamp them to the nearest integer value
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for any follow-up tasks.
Most helpful comment
The factorial of
0is actually1. I've been taught this multiple times and a quick google search says so. So technicallyfactorial(0) -> 1is correct. Safeguards for negative numbers are not in place, but I expect we can easily PR that to fix the problem.