Conversation
Gemfile.lock
Outdated
| rspec (~> 3.0.0.beta2) | ||
|
|
||
| RUBY VERSION | ||
| ruby 2.0.0p648 |
There was a problem hiding this comment.
:-) all of our stuff is on ruby 2.2, so you'll save yourself some trouble later by switching over now
There was a problem hiding this comment.
The Gemfile specified 2.0 so for some reason I switched ruby versions as opposed to updating the Gemfile. In hindsight, that wasn't my most efficient option
| output << "Buzz" if input % 5 == 0 | ||
| output = input if output.empty? | ||
|
|
||
| output |
There was a problem hiding this comment.
I saw Jay use a similar method a few months back for running a FizzBuzz server with Sinatra and I was impressed with his work
| expect(script.speak("BYE")).to eq "NOT SINCE 1964!" | ||
| expect(script.speak("BYE")).to eq "NOT SINCE 1964!" | ||
| expect(script.speak("I guess I'll stay")).to eq "SPEAK UP SONNY!" | ||
| expect(script.speak("BYE")).to eq "NOT SINCE 1964!" |
There was a problem hiding this comment.
There are interesting semi-theoretical questions here about what should be under test in these two examples. If it's really what the third response is, given the setup, then why check the expectation on the first two speak("BYE")'s? If it's all three, then maybe too much is being covered in a single test.
There was a problem hiding this comment.
I agree. I haven't done much work on testing so I'm still working my way through it. It is aiming to test that saying 'BYE' 3 times non consecutively doesn't change the output. I'll update now
lib/deaf_grandma.rb
Outdated
| gets.chomp | ||
| end | ||
|
|
||
| def soft |
There was a problem hiding this comment.
Generally, you want your method names to be verbs - something an instance of the class in question could do.
There was a problem hiding this comment.
^^Maybe that's not right.
Better: you want your method names to be either verbs (something an instance of the class in question could do) -- or else you want your method names to be nouns - a piece of data an instance of the class can tell you about itself
| @bye_counter = 0 | ||
| yell | ||
| end | ||
| end |
There was a problem hiding this comment.
This is nested three levels deep. It's actually not a terrible use-case for deep nesting, but generally that's something to avoid.
lib/deaf_grandma.rb
Outdated
|
|
||
|
|
||
| def speak(input) | ||
| if input.match(/\p{Lower}/) |
There was a problem hiding this comment.
\p{Lower}
Wow, I've never seen this before. Standard approaches would be
input == input.downcaseor
input.match /\A[a-z]*\z/(or something like that, I'm not actually testing these things out.)
But if you can point me to do the relevant docs, I'm always happy to pick up another option.
There was a problem hiding this comment.
I was using an option that was gentle on non-alphabet characters but I achieved the same thing by using upcase as well. \p is a regex way of matching Unicode categories but it's not fully supported across languages (https://www.regular-expressions.info/unicode.html#category). It's supported in Ruby since 1.9
| loop do | ||
| user_input = get_user_input | ||
| p speak(user_input) | ||
| exit if @bye_counter == 3 |
There was a problem hiding this comment.
Since @bye_counter is compared with 3 in two places, consider a new instance method called exiting?.
3 participants
No description provided.