Skip to content

Conversation

@rbasso
Copy link
Contributor

@rbasso rbasso commented Sep 22, 2016

  • Use lyrics from x-common/exercises/food-chain/canonical-data.json.
  • Tests all lines, one by one, and then the entire song.
  • Add stub solution passing the tests.
  • Patch example solution to work with the new tests.

This is a first try at solving #301. When we get this right, we may extend the same logic to beer-song and house.

There is some magic in the test suite to check the lyrics line by line, but I couldn't find a better way to write it.

The stub solution has the complete lyrics in the most raw form possible, so that the student will be forced to refactor everything from zero.

The previous tests suite had an empty line at the end, but that makes no sense and doesn't match the reference file.

- Try to make the code readable.
- Generalize your solution, allowing the lady to swallow other things.

Take you time.
Copy link
Member

Choose a reason for hiding this comment

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

"Take your time"

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. 😄

Anything else?

- Use lyrics from `x-common/exercises/food-chain/canonical-data.json`.
- Tests all lines, one by one, and then the entire song.
- Add stub solution passing the tests.
- Patch example solution to work with the new tests.
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.

strangely(?) I think there is nothing else I would change. Were you expecting more opposition?

Well, you have to keep in mind I haven't done this exercise in any language.

@rbasso
Copy link
Contributor Author

rbasso commented Sep 23, 2016

Thank you for the review, @petertseng! 😄

Were you expecting more opposition?

A little, considering that this is the first exercise where we test two multi-line string, line by line.
Also, I have to admit that my custom zipWith function (go) is at least unusual. 😄

...keep in mind I haven't done this exercise in any language.

In that case I recommend you to try this one when you get the time.

It may seem pointless at first, but I think I learn a few things refactoring it.

@rbasso rbasso merged commit 4c60d45 into exercism:master Sep 23, 2016
@rbasso rbasso deleted the food-chain-rewrite-tests branch September 23, 2016 01:47
@petertseng
Copy link
Member

I spent a little more time on go shouldBe on zip [1 :: Int ..] . lines than go since go was easy enough to understand. I'm not sure I could have done better than go since the goal is to stop at the longer of the lists rather than the shorter.

In that case I recommend you to try this one when you get the time.

I'll start now.

@petertseng
Copy link
Member

Hmm, here's one thing that caught my eye. The failure message -

Failures:

  test/Tests.hs:27: 
  1) food-chain.song matches lines
       expected: Just (2,"I don't know why she swallowed the fly. Perhaps she'll die.")
        but got: Just (2,"Hmm, I messed up")

It may not be clear to the reader that this means "there was a mismatch at line 2", maybe?

@rbasso
Copy link
Contributor Author

rbasso commented Sep 23, 2016

Yeah! I know... it is far from good.

The problem is that, to compare with a possibly missing line, we need to compare Maybes. I don't know how to customize an error message... I need to learn more about hspec.

About the line number, we can have it in the data compared, like we did, or in the test description. In the later case, it would create one test case for each line, and that creates a lot of output.

I agree that it deserves to be improved, but I still don't know exactly how. 😕

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