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

Issue33 math func #39

Merged
merged 22 commits into from
Jul 9, 2018
Merged

Issue33 math func #39

merged 22 commits into from
Jul 9, 2018

Conversation

CanBerker
Copy link
Collaborator

All mathematical functions have been implemented.

When implementing "round" and "roundhalftoeven" I defined a rounding function in the "RoundFunctionIterator" which is used by both iterators. If you have suggestions for a better practise or location to define the function please let me know.

With the recently added functions the functions folder is getting quite crowded. Moving them to a "Numeric" folder or multiple folders such as "trigonometric", "exponential" and so on can be beneficial for organization.

Copy link
Collaborator

@wscsprint3r wscsprint3r left a comment

Choose a reason for hiding this comment

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

Looks good to me, just one comment.

* Decimal Formatting Solution
* https://stackoverflow.com/questions/153724/how-to-round-a-number-to-n-decimal-places-in-java
*/
public static Double getRoundedResult(Double value, Integer precision, RoundingMode rm) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This needs to be moved to some utility class, it doesn't make sense to have it in the Iterator as a static method.

It would be good if you could find a pre-existing solution since rounding of doubles is not an uncommon problem.
This may or may not help you:
https://commons.apache.org/proper/commons-math/javadocs/api-3.3/org/apache/commons/math3/util/Precision.html

@ghislainfourny
Copy link
Member

This is awesome work, Can. I have added my minor comments inline.

@@ -0,0 +1,7 @@
(:JIQS: ShouldRun; Output="( 0.0, 2.0, 2.0, 3567.81, 0.0, 35600.0)" :)
roundhalftoeven(0.5),
Copy link
Member

Choose a reason for hiding this comment

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

the name should be round-half-to-even

@@ -0,0 +1,6 @@
(:JIQS: ShouldRun; Output="( 0.0, -0.0, 1000.0)" :)
sqrt(0.0e0),
Copy link
Member

Choose a reason for hiding this comment

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

It would be great to add a test for sqrt of a negative number (double-checking which error should be returned in the specification)

@CanBerker CanBerker merged commit 07b5928 into alpha-0.9.2 Jul 9, 2018
@CanBerker CanBerker deleted the ISSUE33_math_func branch July 9, 2018 16:46
AndreaRinaldi1 pushed a commit to AndreaRinaldi1/rumble that referenced this pull request Oct 12, 2019
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.

None yet

3 participants