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

Finished challenge - David and Luke #98

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

levine1726
Copy link

No description provided.

@@ -19,8 +19,21 @@ def run!


def speak(input)

Choose a reason for hiding this comment

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

Nice job solving it! Does this method look like it's got a lot of decisions being made? Think about how you might be able to refactor this to make the code paths a little easier to follow.

Copy link
Author

Choose a reason for hiding this comment

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

Refactored the code to reduce if statements and minimize code block length

end

it "EXTRA CREDIT: How would you test yelling BYE?" do
#implement your test here
expect(script.speak("BYE")).to eq "NOT SINCE 1964!"

Choose a reason for hiding this comment

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

We only want Grandma to respond if you have yelled 3 times at her. Think about how you could write a test to make sure the first two times you say bye, she doesn't respond.

Copy link
Author

Choose a reason for hiding this comment

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

Made fix to return empty string

@bye_counter += 1
""
else
@bye_counter = 0

Choose a reason for hiding this comment

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

You don't need to reset the bye counter here and see if you can get the program to exit after the 3rd bye.

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