-
Notifications
You must be signed in to change notification settings - Fork 21
Add refactoring #1
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
base: main
Are you sure you want to change the base?
Conversation
| @guesses = [] | ||
| @number_correct = 0 | ||
| @current_card = 0 | ||
| @card_position = 0 |
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.
This naming better represents how this value is being used, which is as an index position.
| @deck.cards[@card_position] | ||
| end | ||
|
|
||
| def delay_output(seconds) |
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.
More dynamic, better named, and testable.
| end | ||
|
|
||
| def check_guess(guess) | ||
| @number_correct += 1 if guess.correct? |
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.
Less lines of code, testable, and more readable.
|
|
||
| def percent_correct | ||
| (@number_correct.to_f / deck.cards.length * 100).to_i | ||
| (@number_correct.to_f / deck.count * 100).to_i |
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.
Fixing indentation is part of refactoring/cleanup.
| end | ||
|
|
||
| def game | ||
| def start_game |
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.
Better method naming.
| puts "Question: #{card.question}" | ||
| input = gets.chomp | ||
| record_guess(input) | ||
| puts "This is card number #{@card_position + 1} out of #{deck.count}" |
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.
Indentation matters for readability.
| round = Round.new(deck) | ||
|
|
||
| assert_equal 1, round.delay_output(1) | ||
| assert_equal 2, round.delay_output(2) |
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.
We discovered quite a bit about the sleep method by writing these tests.
| assert_equal 2, round.delay_output(2) | ||
| end | ||
|
|
||
| def test_check_guess |
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.
Whenever there are multiple scenarios based on a condition, we want to test both of them.
| end | ||
|
|
||
| def input | ||
| # More on why I do this with the mocks and stubs lesson |
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.
We always want to isolate user input.
| def start | ||
| puts "Welcome! You're playing with #{deck.cards.count} cards" | ||
| sleep 1.5 | ||
| puts "Welcome! You're playing with #{deck.count} cards" |
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 built a count method on the deck class, I should use it.
No description provided.