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

pascals-triangle: Rewrite tests to use hspec with fail-fast. #287

Merged
merged 1 commit into from
Sep 15, 2016
Merged

pascals-triangle: Rewrite tests to use hspec with fail-fast. #287

merged 1 commit into from
Sep 15, 2016

Conversation

rbasso
Copy link
Contributor

@rbasso rbasso commented Sep 8, 2016

  • Rewrite tests to use hspec.
  • Rename function triangle to rows
  • Change tests do expect row to count lines starting at zero.
  • Rewrite the stub solution to be more concise and match the new test suite.

Considering that there is a ongoing discussion in exercism/problem-specifications#362, I think we should wait it before merging this PR.

Related to #211.

@petertseng
Copy link
Member

remind when that discussion concludes, I think. I may forget otherwise. I have no problems with what I see in here so far

@rbasso
Copy link
Contributor Author

rbasso commented Sep 14, 2016

Tests rewritten to match x-common.

The only relevant divergence is that, for negative numbers, I decided to return an empty list, mimicking the behavior of take. We already have exercises using Maybe, so I though it would be a good idea, and it also allows simpler implementations.

, ("two rows" , 2, [[1], [1, 1] ])
, ("three rows" , 3, [[1], [1, 1], [1, 2, 1] ])
, ("four rows" , 4, [[1], [1, 1], [1, 2, 1], [1, 3, 3, 1]])
, ("negative rows", 0, [ ]) ]
Copy link
Member

Choose a reason for hiding this comment

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

you didn't actually put a negative number here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My stupid mistake. Have no idea why I changed that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed! Thanks for catching that, @petertseng! 😄

@petertseng
Copy link
Member

I'm debating whether to say "maybe the test should explain why negative rows is empty because it's like take", but maybe that will hint too strongly to people about using take! So maybe no explanation needed? What say you?

- Rewrite tests to use `hspec`.
- Change tests to match `x-common`.
- Rewrite the stub solution to be simpler and match the new test suite.
@petertseng
Copy link
Member

Yup. 👍

@rbasso
Copy link
Contributor Author

rbasso commented Sep 15, 2016

... maybe the test should explain why negative rows is empty because it's like take ...

Both take and drop silently ignore invalid inputs. This can be seem as a feature or as a bad design, but I don't have a strong opinion on this.

Considering that we have this behavior in Prelude, I don't think we have to explain anything, unless there is a consensus that take and drop have an undesirable specification.

@rbasso rbasso merged commit 9fc98d7 into exercism:master Sep 15, 2016
@rbasso rbasso deleted the hspec-pascals-triangle branch September 15, 2016 03:30
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.

2 participants