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

house: transform output to array of whole sentence #1201

Merged
merged 3 commits into from Mar 3, 2018

Conversation

mrcfps
Copy link
Contributor

@mrcfps mrcfps commented Feb 27, 2018

Closes #1200.

  • Bump version
  • Remove redundant reasons for separating a verse into strings
  • Transform the output to the same format as twelve-days

mRcfps added 2 commits February 27, 2018 15:54
* Bump version
* Remove redundant reasons for separating a verse into strings
* Transform the output to the same format as twelve-days
"comments": [
"JSON doesn't allow for multi-line strings, so all verses are presented ",
Copy link
Member

Choose a reason for hiding this comment

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

Any reason to remove this comment? As far as I can tell, it still holds true when multiple verses are recited. The food-chain canonical data also has this comment.

@petertseng
Copy link
Member

I have determined that under the following diff to the verifier, this JSON file verifies.

diff --git i/exercises/house/verify.rb w/exercises/house/verify.rb
index ea4456a1..c013f33d 100644
--- i/exercises/house/verify.rb
+++ w/exercises/house/verify.rb
@@ -17,7 +17,7 @@ THINGS = [
 ].map(&:freeze).freeze
 
 def verse(n)
-  "This is #{THINGS.last(n).join}.".lines.map(&:chomp)
+  "This is #{THINGS.last(n).join}.".lines.join.tr("\n", ' ')
 end
 
 module Intersperse refine Enumerable do
@@ -29,7 +29,7 @@ end end
 using Intersperse
 
 def verses(a, b)
-  (a..b).map { |n| verse(n) }.intersperse('').flatten(1)
+  (a..b).map { |n| verse(n) }.flatten(1)
 end
 
 json = JSON.parse(File.read(File.join(__dir__, 'canonical-data.json')))

This comment only confirms that fffe9f5 is consistent. I make no judgment (please do not ask me to) on whether the change is desirable.

@petertseng
Copy link
Member

Person who merges: Please do remember to squash. Thank you.

@ErikSchierboom ErikSchierboom merged commit e8a357c into exercism:master Mar 3, 2018
@ErikSchierboom
Copy link
Member

Squashed and merged. Thanks @mrcfps!

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.

None yet

3 participants