Add helpers.math._factorize

[benmccann]

This is splitting off part of #6274 to make that PR smaller and easier to review. I plan to reopen that PR soon

By exporting this as part of helpers.math I'm able to add tests for this method and it also helps organize the code. The helpers.math file is also being added in #6343 in order to add a helpers.math.log10

[nagix]

I learned a bitwise OR (|) on floating-point numbers converts them to 32-bit signed integers 🙂. Wouldn't it be better to have test cases for invalid values?

[benmccann]

Thanks for taking a look. I've added input validation per your suggestion

[nagix]

Thank you, but I think it would be enough to return an empty array and checking that.

[benmccann]

If we return an empty array then it will be much harder to figure out why the code is not working as expected. We should never encounter a non-positive integer as input. If we do then I want to know as soon as possible so that I can find the bug causing the bad input rather than hiding the error and making it harder to find what's going wrong. Here's a detailed article about the concept: https://www.martinfowler.com/ieeeSoftware/failFast.pdf

[nagix]

That's true. However, it is an internal method and not supposed to be used by users. I feel it's too much to check and throw an error in it.

[benmccann]

@etimberg @kurkle do you guys have thoughts about which is the better direction to go?

[benmccann]

@nagix I had another idea. What if I return an empty array, but log a warning with console.warn? I think that would both satisfy your concern of not failing the whole chart and mine of not silently ignoring an error.

[kurkle]

I do agree throw is a bit much.
You could just remove the check, Math.sqrt will return NaN on invalid input and function will return empty array. I think that's fine behavior for invalid input to internal helper.

I don't think adding console.warn is good idea either, its just bloat in the lib.

[benmccann]

Ok. I've gone ahead and removed. Thanks again for taking a look at this