Skip to content

Herons : Changed the signature of the function #4686

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

Merged
merged 13 commits into from
Oct 23, 2023

Conversation

satyabarghav
Copy link
Contributor

  • I have read CONTRIBUTING.md.
  • This pull request is all my own work -- I have not plagiarized it.
  • All filenames are in PascalCase.
  • All functions and variable names follow Java naming conventions.
  • All new algorithms have a URL in their comments that points to Wikipedia or other similar explanations.

I have already raised this issue here
The required changes were made to make it work with even larger set of data types like double and float and further documentation was also added ro ease the readability and understandability of the code.

@satyabarghav
Copy link
Contributor Author

satyabarghav commented Oct 6, 2023

closes #4685

@satyabarghav satyabarghav force-pushed the master branch 3 times, most recently from 5f4f5d5 to 2df4690 Compare October 7, 2023 06:29
@vil02 vil02 self-assigned this Oct 21, 2023
@vil02
Copy link
Member

vil02 commented Oct 21, 2023

Fixes #4685.

@satyabarghav satyabarghav force-pushed the master branch 4 times, most recently from 22724b5 to 86f96c4 Compare October 22, 2023 14:33
@satyabarghav
Copy link
Contributor Author

Made the requested changes, I request the Reviewer(s) to please tag this as hacktoberfest-accepted.

@satyabarghav satyabarghav force-pushed the master branch 2 times, most recently from 5840c3e to 383146b Compare October 22, 2023 14:50
@vil02
Copy link
Member

vil02 commented Oct 22, 2023

Made the requested changes, I request the Reviewer(s) to please tag this as hacktoberfest-accepted.

This is not needed, since this repository has hacktoberfest tag. The website will recognize your contribution.

@vil02
Copy link
Member

vil02 commented Oct 23, 2023

@satyabarghav there was a subtle error in the check if all of the provided sidelights are positive, namely, the tests:

Assertions.assertThrows(IllegalArgumentException.class, () -> { HeronsFormula.herons(1, 1, 0); });
Assertions.assertThrows(IllegalArgumentException.class, () -> { HeronsFormula.herons(1, 0, 1); });
Assertions.assertThrows(IllegalArgumentException.class, () -> { HeronsFormula.herons(0, 1, 1); });

did not pass.

Always add tests.

To speed up the whole process I allowed myself to push a commit into your branch - I hope you do not mind.

@vil02 vil02 merged commit 9dae389 into TheAlgorithms:master Oct 23, 2023
@satyabarghav
Copy link
Contributor Author

Cool, thanks for being with me :D

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