-
Notifications
You must be signed in to change notification settings - Fork 82
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
Working Super Fizz Buzz and Deaf Grandma drills and rspec tests. #85
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good. Check out some of these comments.
@@ -1,13 +1,29 @@ | |||
class SuperFizzBuzz | |||
|
|||
def run(input) | |||
output = "" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Take a look at .tap
.
https://ruby-doc.org/core-2.5.0/Object.html#method-i-tap
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not seeing it. How would I use .tap here to avoid initializing output to an empty string?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could use it to simplify your code a bit here because it returns itself. You'll see .tap
used quite a bit. Let's say...
def run_service
api_service.tap do |s|
s.prepare_data
s.send_emails
s.log_something
end
end
In this particular case, you could...
"".tap do |fizz_buzz_string|
fizz_buzz_string << "Fizz" if condition
...
end
lib/fizzbuzz.rb
Outdated
|
||
end | ||
if divBy3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
snakecase_is_preferred.
https://github.com/bbatsov/ruby-style-guide#snake-case-symbols-methods-vars
lib/fizzbuzz.rb
Outdated
|
||
if divBy3 || divBy5 | ||
return output |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you need an explicit return here?
https://stackoverflow.com/questions/1064159/implicit-return-values-in-ruby
lib/fizzbuzz.rb
Outdated
if divBy3 || divBy5 | ||
return output | ||
else | ||
return input |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or here?
lib/deaf_grandma.rb
Outdated
if input == input.upcase | ||
#YELL | ||
if input == "BYE" | ||
@bye_counter = @bye_counter + 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please review @jaybobo .