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

[#5] Num laws for Size #32

Merged
merged 3 commits into from
May 3, 2020
Merged

[#5] Num laws for Size #32

merged 3 commits into from
May 3, 2020

Conversation

chshersh
Copy link
Contributor

@chshersh chshersh commented May 3, 2020

Part of #5

Checks Num instance laws for Size. I've also added some simple check on integer overflow.

@chshersh chshersh added the tests label May 3, 2020
@chshersh chshersh self-assigned this May 3, 2020
Copy link
Contributor

@hint-man hint-man bot left a comment

Choose a reason for hiding this comment

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

Do you know why your PR is still not approved? Because I chose not to approve it. But they will.

test/Test/Slist/Size.hs Show resolved Hide resolved
test/Test/Slist/Size.hs Show resolved Hide resolved
test/Test/Slist/Size.hs Show resolved Hide resolved

additionAssotiavity :: Property
additionAssotiavity = hedgehog $ do
x <- forAll genSize
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion: Reduce duplication

Suggested change
x <- forAll genSize
Combine with test/Test/Slist/Size.hs:75:5

test/Test/Slist/Size.hs Show resolved Hide resolved
x <- forAll genSize

fromInteger 1 * x === x
x * fromInteger 1 === x
Copy link
Contributor

Choose a reason for hiding this comment

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

Warning: Redundant fromInteger

Suggested change
x * fromInteger 1 === x
x * 1 === x

Copy link
Member

@vrom911 vrom911 left a comment

Choose a reason for hiding this comment

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

This is amazing 😮 😎 Thank you, @chshersh !

Comment on lines +72 to +78
(Size x) * (Size y)
| x == 0 || y == 0 = 0
| otherwise =
let result = x * y in
if x == result `div` y
then Size (x * y)
else Infinity -- multiplication overflow
Copy link
Member

Choose a reason for hiding this comment

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

🐺 👍

@@ -1 +1 @@
resolver: nightly-2019-11-08
resolver: nightly-2020-05-01
Copy link
Member

Choose a reason for hiding this comment

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

It's been a while 😸

@vrom911 vrom911 merged commit 300a57a into master May 3, 2020
@vrom911 vrom911 deleted the chshersh/5-Property-tests branch May 3, 2020 15:15
@chshersh chshersh added this to the 0.2.0.0: Richer interface milestone Jun 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants