Skip to content

Commit

Permalink
refactor: remove hardcoded constant
Browse files Browse the repository at this point in the history
PR-URL: #2970
Reviewed-by: Athan Reines <[email protected]>
  • Loading branch information
gunjjoshi authored Oct 1, 2024
1 parent d0ebce3 commit aa94a37
Showing 1 changed file with 4 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,7 @@ var floor = require( '@stdlib/math/base/special/floor' );
var abs = require( '@stdlib/math/base/special/abs' );
var FLOAT64_MAX = require( '@stdlib/constants/float64/max' );
var PINF = require( '@stdlib/constants/float64/pinf' );


// VARIABLES //

var MAX_FACTORIAL = 170; // TODO: consider packaging as constant
var FLOAT64_MAX_SAFE_NTH_FACTORIAL = require( '@stdlib/constants/float64/max-safe-nth-factorial' ); // eslint-disable-line id-length


// FUNCTIONS //
Expand Down Expand Up @@ -181,10 +177,10 @@ function fallingFactorial( x, n ) {
}
if ( x < 0.5 ) {
// Computing `1 + x` will throw away digits, so split up calculation...
if ( n > MAX_FACTORIAL-2 ) {
if ( n > FLOAT64_MAX_SAFE_NTH_FACTORIAL-2 ) {
// Given a ratio of two very large numbers, we need to split the calculation up into two blocks:
t1 = x * fallingFactorial( x-1.0, MAX_FACTORIAL-2 );
t2 = fallingFactorial( x-MAX_FACTORIAL+1.0, n-MAX_FACTORIAL+1 );
t1 = x * fallingFactorial( x-1.0, FLOAT64_MAX_SAFE_NTH_FACTORIAL-2 ); // eslint-disable-line max-len
t2 = fallingFactorial( x-FLOAT64_MAX_SAFE_NTH_FACTORIAL+1.0, n-FLOAT64_MAX_SAFE_NTH_FACTORIAL+1 ); // eslint-disable-line max-len
if ( FLOAT64_MAX/abs(t1) < abs(t2) ) {
return PINF;
}
Expand Down

1 comment on commit aa94a37

@stdlib-bot
Copy link
Contributor

Choose a reason for hiding this comment

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

Coverage Report

Package Statements Branches Functions Lines
math/base/special/falling-factorial $\color{red}239/263$
$\color{green}+90.87\%$
$\color{red}28/33$
$\color{green}+84.85\%$
$\color{green}2/2$
$\color{green}+100.00\%$
$\color{red}239/263$
$\color{green}+90.87\%$

The above coverage report was generated for the changes in this push.

Please sign in to comment.