Skip to content

algorithm: AbsoluteValue#31

Merged
raklaptudirm merged 1 commit into
TheAlgorithms:masterfrom
orangegrove1955:add-absolute-value
Oct 7, 2022
Merged

algorithm: AbsoluteValue#31
raklaptudirm merged 1 commit into
TheAlgorithms:masterfrom
orangegrove1955:add-absolute-value

Conversation

@orangegrove1955

Copy link
Copy Markdown
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

@appgurueu appgurueu left a comment

Copy link
Copy Markdown
Contributor

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
Copy Markdown
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
Copy Markdown
Member

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

@appgurueu

Copy link
Copy Markdown
Contributor

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

@raklaptudirm raklaptudirm left a comment

Copy link
Copy Markdown
Member

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