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.

Sign up for GitHub

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

Jump to bottom

Clarify documentation and properties of splitCoin. #31

Merged
merged 11 commits into from
Apr 7, 2020
Merged

Clarify documentation and properties of splitCoin. #31

merged 11 commits into from
Apr 7, 2020

Conversation

jonathanknowles
Copy link
Contributor

Related Issues

#18
#21
#22

Summary

This PR:

  • Clarifies the documentation of splitCoin, adding examples.
  • Tests that splitCoin preserves the total sum.
  • Tests that splitCoin only produces valid coins.

@jonathanknowles jonathanknowles self-assigned this Apr 6, 2020
paweljakubas
paweljakubas reviewed Apr 6, 2020
View reviewed changes
src/Cardano/Fee.hs Outdated Show resolved Hide resolved
paweljakubas
paweljakubas approved these changes Apr 6, 2020
View reviewed changes
Copy link
Contributor

@paweljakubas paweljakubas left a comment

Choose a reason for hiding this comment

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

looks, fine. very nice property test, especially data coverage

jonathanknowles
jonathanknowles commented Apr 6, 2020
View reviewed changes
src/Cardano/Fee.hs Outdated Show resolved Hide resolved
@jonathanknowles
Test that SplitCoinData generation is valid.
5a4149b
@jonathanknowles
Add shrinking for SplitCoinData.
7f61a54 
Additionally, test that shrinking always generates valid data.
@jonathanknowles
Test that splitCoin splits coins fairly.
3c3493b 
When it's not possible to divide a coin evenly, each integral coin value
in the resulting list should always be within unity of the ideal
unrounded result.

This change adds a test to verify this property.
@jonathanknowles
Cover case where coin to split is smaller than length of list.
90b7dbf
@jonathanknowles
Clarify comment about even coin division.
973b488
@jonathanknowles jonathanknowles merged commit 7295c18 into master Apr 7, 2020
@jonathanknowles jonathanknowles deleted the jonathanknowles/split-coin branch April 7, 2020 04:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@paweljakubas paweljakubas paweljakubas approved these changes

Assignees

@jonathanknowles jonathanknowles

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jonathanknowles @paweljakubas