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

algorithm: AbsoluteValue #31

Merged

Conversation

orangegrove1955
Copy link
Contributor

Includes changes related to Issue: #10

Included a new maths function to calculate the absolute value of numeric values, as well as associated tests

Copy link
Contributor

@appgurueu appgurueu left a comment

Choose a reason for hiding this comment

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

The tests & docs are overkill for such a trivial function.

@orangegrove1955
Copy link
Contributor Author

@appgurueu I tried to include all examples for test cases, so someone completely new to the world of Typescript / Javascript would be able to easily understand what the function does with no prior knowledge. If you like, I can remove some of the tests / documentation, just let me know which ones you would prefer to keep, and which you think should be removed

@raklaptudirm
Copy link
Member

Even though it is overkill, I guess there is no harm in keeping the tests.

@appgurueu
Copy link
Contributor

I don't think abs is that hard to understand - even for someone new to JS.

Copy link
Member

@raklaptudirm raklaptudirm left a comment

Choose a reason for hiding this comment

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

LGTM. @appgurueu I think there is no harm in keeping the tests. Inform me if you disagree.

@raklaptudirm raklaptudirm changed the title Added AbsoluteValue and its associated tests algorithm: AbsoluteValue Oct 7, 2022
@raklaptudirm raklaptudirm merged commit eb8261f into TheAlgorithms:master Oct 7, 2022
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