Skip to content

Speed up Number#numberBetween for int values - #838

Merged
snuyanzin merged 1 commit into
datafaker-net:mainfrom
snuyanzin:test
May 20, 2023
Merged

Speed up Number#numberBetween for int values#838
snuyanzin merged 1 commit into
datafaker-net:mainfrom
snuyanzin:test

Conversation

@snuyanzin

@snuyanzin snuyanzin commented May 20, 2023

Copy link
Copy Markdown
Collaborator

The test to check

    @Test
    void test() {
        for (int i = 0; i < 100_000_000; i++) {
            faker.number().numberBetween(-1000_000_000, 2000_000_000);
        }
    }

before the change it takes about 20x times longer to complete ...

the main idea is in case of overloading the limit use fallback to long instead of fallback to BigDecimal

@what-the-diff

what-the-diff Bot commented May 20, 2023

Copy link
Copy Markdown

PR Summary

  • Updated method parameters
    The method 'numberBetween(int min, int max)' now uses the 'final' keyword for its parameters, ensuring they remain constant throughout the method.

  • Optimized variable calculation
    A new variable 'amplitude' of type long is declared and initialized to avoid duplicate calculations, improving efficiency.

  • Enhanced input validation
    The input validation for the method now checks both minimum and maximum bounds to ensure a valid range, allowing it to work with positive and negative numbers.

  • Improved return value handling
    The method returns an integer value casted from a double to better handle large ranges and potential overflow, without the need for throwing exceptions.

@snuyanzin
snuyanzin merged commit 2bade94 into datafaker-net:main May 20, 2023
@bodiam

bodiam commented May 20, 2023

Copy link
Copy Markdown
Contributor

Do the final parameters make any difference here?

@snuyanzin

Copy link
Copy Markdown
Collaborator Author

right now no difference
the only reason I made it it is kind of self-protection from possible reassigning values in methods in future.

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.

2 participants