Skip to content

parallel-letter-frequency: Rewrite tests to use hspec with fail fast. #299

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
merged 1 commit into from
Sep 18, 2016
Merged

parallel-letter-frequency: Rewrite tests to use hspec with fail fast. #299

merged 1 commit into from
Sep 18, 2016

Conversation

rbasso
Copy link
Contributor

@rbasso rbasso commented Sep 16, 2016

  • Rewrite tests to use hspec.
  • Move hints to HINTS.md.

Related to #211.

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.

Yup, seems like a faithful translation 👍

I wonder whether some people will now wonder what the Int arg is if they don't read the README. However the status quo before 1eaff79 was that a stub solution didn't exist at all, so things have at least not be made worse.

If there were a concrete solution to be made, it would be maybe name the args: frequency nWorkers texts = undefined

But maybe that will be too prescriptive, if someone wants to write it eta-reduced (I don't know if anyone will)

@petertseng
Copy link
Member

Hmm. guess you can never edit a review summary comment. That's interesting. I was going to edit to link to 1eaff79 .

- Rewrite tests to use `hspec`.
- Move hints to `HINTS.md`.
- Add variable names to the function in the *stub solution*.
@rbasso
Copy link
Contributor Author

rbasso commented Sep 18, 2016

I wonder whether some people will now wonder what the Int arg is if they don't read the README.

We don't have to save the lazy ones, but I think it is nice to save the users the work of looking at the README.md.

If there were a concrete solution to be made, it would be maybe name the args: frequency nWorkers texts = undefined

Granted! 😁

But maybe that will be too prescriptive, if someone wants to write it eta-reduced (I don't know if anyone will)

I don't see that as a problem, because the stub solution is just a suggestion, and I expect the serious student to rewrite it multiple times. What usually bothers me is that I'm never sure about how to name the variables - workers, n, nWorkers, texts, ts, xs - so I often delegate that choice to the users.

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.

👍

@rbasso rbasso merged commit 8013662 into exercism:master Sep 18, 2016
@rbasso rbasso deleted the hspec-parallel-letter-frequency branch September 18, 2016 00:34
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