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

bowling: Add JSON test data #240

Merged
merged 1 commit into from
May 1, 2016
Merged

Conversation

bernardoamc
Copy link
Contributor

No description provided.

@kotp
Copy link
Member

kotp commented Apr 28, 2016

Looks good to me. There are messages here, looking at the example.rb those would be expected to be feedback when errors aren't raised, but generic enough to not drive an implementation too much.

@IanWhitney
Copy link
Contributor

IanWhitney commented Apr 29, 2016

I never figured out how to score bowling, so keep that in mind while I ask some questions

Do all the tests cover a unique case? For example, what parts of the students' implementation has to change between this test and this one?

If nothing has to change, then I suggest getting rid of one of those tests. Tests are better if they cover a new, possibly unexpected, case. So what are the bare minimum number of tests that will completely exercise the concept of scoring a bowling game?

I'm not sure about these two tests. Why can't I get a score (0, presumably) at the start of the game? Or in the 7th frame? I don't bowl often, but I know that those machines can tell me my score as I go along.

@bernardoamc
Copy link
Contributor Author

@IanWhitney I think we can indeed remove the should be able to score multiple frames test since we have other cases covering this.

I implemented it this way because the exercise specified that we should take the score only at the end of the game. I think it was done this way to easy the solution, since we can't take score of the game all the time (if we have a strike or spare we need to wait for the next frames to compute it correctly).

@IanWhitney
Copy link
Contributor

I think we can indeed remove the should be able to score multiple frames test since we have other cases covering this.

Looking at Uncle Bob's powerpoint (powerpoint?!), his kata has just 5 tests. That might be enough to cover all the edge cases. But I leave that up to those who know more about bowling.

I implemented it this way because the exercise specified that we should take the score only at the end of the game. I think it was done this way to easy the solution, since we can't take score of the game all the time (if we have a strike or spare we need to wait for the next frames to compute it correctly).

Yeah, Uncle Bob's kata does specify that you can't score until after the 10th frame. It does avoid some complexity, which is probably fine.

@kotp
Copy link
Member

kotp commented Apr 29, 2016

Should we merge this into the wild and let folks make generators for it, and see how it pans out? It may very well be that the number of tests will dwindle as we go.

I believe the file itself is written well, very much patterned for simplicity.

@bernardoamc
Copy link
Contributor Author

Looking at Uncle Bob's powerpoint (powerpoint?!), his kata has just 5 tests. That might be enough to cover all the edge cases.

I like tests that pass an idea of progression. You start with the simple cases than you start testing the edge cases, even if they overlap a little. Some tests are also about constraints, which I think are important and were not implemented in Uncle Bob's kata (probably to avoid complexity).

@kytrinyx
Copy link
Member

About the redundant tests: are they purely redundant or do they potentially help people triangulate?

@bernardoamc
Copy link
Contributor Author

About the redundant tests: are they purely redundant or do they potentially help people triangulate?

I my opinion these tests are guiding the thinking process.

  1. Score a frame, ok now what?
  2. Score multiple frame, nice, what's missing?
  3. Maybe scoring a spare or strike?
  4. And it keeps going.

I like these kind of tests, but I know that they are sometimes redundant and not everyone agrees with them.

@kytrinyx
Copy link
Member

kytrinyx commented May 1, 2016

I think that as long as it's intentional I'm happy with it. Let's do this!

@kytrinyx kytrinyx merged commit 09a69d0 into exercism:master May 1, 2016
emcoding pushed a commit that referenced this pull request Nov 19, 2018
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.

4 participants