Skip to content

rational-numbers Add additional test cases #1914

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

Conversation

neduard
Copy link
Contributor

@neduard neduard commented Jan 8, 2022

Fixes #1909 . Any feedback welcome!

Note I have not ran the schema check. Relying on CI.

"r": [-3, 5],
"n": -2
},
"expected": [-25, 9]
Copy link
Member

Choose a reason for hiding this comment

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

as I understand it, the result should be positive here, because a negative times a negative (we square the number) will be positive.

However, if we had taken the power -3, it would remain negative.

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"expected": [-25, 9]
"expected": [25, 9]

We could have cases for both n = -2 and n = -3 too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

D'oh 🤦 you are right! Fixed the test and added a new one with an odd negative power.

"r": [-3, 5],
"n": -2
},
"expected": [-25, 9]
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"expected": [-25, 9]
"expected": [25, 9]

We could have cases for both n = -2 and n = -3 too.

Copy link
Member

@petertseng petertseng left a comment

Choose a reason for hiding this comment

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

OK. But, you would do better to use a word in https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue so that it automatically closes the issue

@SaschaMann SaschaMann linked an issue Jan 9, 2022 that may be closed by this pull request
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.

rational-numbers missing test case for negative rational exponent
4 participants