The Wayback Machine - https://web.archive.org/web/20201103183803/https://github.com/TheAlgorithms/Java/pull/1683
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

add BigInteger implementation to Factorial.java #1683

Open
wants to merge 1 commit into
base: master
from

Conversation

@antailbaxt3r
Copy link

@antailbaxt3r antailbaxt3r commented Oct 7, 2020

Describe your change:

The previous implementation of Math/Factorial.java involved usage of long datatypes that have large storage, but it cannot even handle 100 factorial. So to solve this, I have implemented the algorithm using java.math.BigInteger which can store values upto 2^32-1 digits.

BigInteger Oracle Documentation

  • Add an algorithm?
  • Fix a bug or typo in an existing algorithm?
  • Documentation change?

References

Issue #1682 - Add BigInteger Implementation to Factorial.java

Checklist:

  • I have read CONTRIBUTING.md.
  • This pull request is all my own work -- I have not plagiarized.
  • I know that pull requests will not be merged if they fail the automated tests.
  • This PR only changes one algorithm file. To ease review, please open separate PRs for separate algorithms.
  • All new Java files are placed inside an existing directory.
  • All filenames are in all uppercase characters with no spaces or dashes.
  • All functions and variable names follow Java naming conventions.
Copy link

@ShivamShekhar1997 ShivamShekhar1997 left a comment

The BigInteger implementation seems to be a proper one

@sanjana1604
Copy link

@sanjana1604 sanjana1604 commented Oct 15, 2020

@ShivamShekhar1997 Is the issue is still open and if yes can you tell me what needs to be done?

@ShivamShekhar1997
Copy link

@ShivamShekhar1997 ShivamShekhar1997 commented Oct 15, 2020

@ShivamShekhar1997 Is the issue is still open and if yes can you tell me what needs to be done?

No @sanjana1604 I guess the implementation is a good one and I don't see the need of any changes, just needs to be merged, thanks anyways!

@antailbaxt3r
Copy link
Author

@antailbaxt3r antailbaxt3r commented Oct 16, 2020

If the PR is valid please get it merged, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.