Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[NUMBERS-133] Use iteration algorithm from bounded trial division in Primes.nextPrime(int) #68

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

Schamschi
Copy link
Contributor

I'm not sure if "potentialPrimes" is a good name for the method that generates the iterator, because it doesn't reveal anything about the purpose of the argument (a lower bound). However, "potentialPrimesBeyond" or something similar would be wrong, because the argument itself is also included in the possible range of results. The most descriptive name would be "potentialPrimesGreaterThanOrEqualTo", but this is horribly verbose. "potentialPrimesNotSmallerThan" is only slightly less verbose, and it contains a negation, which is confusing. So I've left it at "potentialPrimes", but any better suggestions are welcome. At least it's a package-private method, so the method name could still be changed any time in the future.

@coveralls
Copy link

coveralls commented Jul 24, 2019

Coverage Status

Coverage increased (+0.02%) to 94.276% when pulling c42eb1a on Schamschi:NUMBERS-133 into 08962e8 on apache:master.

@aherbert
Copy link
Contributor

How about potentialPrimesGTE? The javadoc can just contain (GTE) in brackets after the greater than or equal to text.

GTE

@Schamschi
Copy link
Contributor Author

@aherbert That sounds like I good idea. I had also thought about using an abbreviation, but I only considered the one from the LaTeX tag "geq", which would probably have been too LaTeX specific. I don't know how common "gte" is, but chances are it will still prevent more confusion than it causes. Thanks.

@aherbert
Copy link
Contributor

@Schamschi The merge of this has been missed. Would you care to rebase this on the current master?

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