StevenThiesfeld / 2015-01-29-rps

0 stars 0 forks source link

Reflections #3

Open ioscowboi opened 9 years ago

ioscowboi commented 9 years ago

I have not had much luck uploading videos, so you'll only see text responses from me today:

Steven, you have written an excellent program. Most of my comments will be user interface related.

a) rpsdriver.rb I think it would look better if the different options at the beginning of the game were on one line. So for example, For Single Game type 1 For Tourney type 2

why not just say “Enter a 1 or a 2: Single Game = 1, Tournament = 2”

then continue your code as normal!

b) rps_player.rb I love the “Username, pick your move: [“rock”…] on line 120 of rps_player.rb. However, the spacing was a little hard to read. If I were you, I’d go back and add a puts between each line of your code to make it easier to read what is happening on the screen…

c) rpsdriver.rb There is no interface for a tiebreaker. I feel this should be there. The program knows to keep running until the end of the program, but the user won’t necessarily know that the desired number of rounds has been reached, and the pressure is on! I would recommend adding a “Tiebreaker!” puts starting on line 53 (make space for it) of rps_game.rb. ex:

line 45:

counter = @somevariablenameyouchoose_basedon_numberofrounds_userchooses

skip to line 52:

else puts “tie” end

start new code here:

counter -= 1 if counter == 0 puts “Tiebreaker!” end

continue code as usual!

code will print “Tiebreaker1” until someone wins

d) rps_player.rb Regarding rock paper scissors, not rock paper scissors lizard spoke:

Shouldn’t player 1’s move be called after player 2 plays? That way player 2 can’t see the move. Maybe you could make the make_move method into two separate methods (location: rps_player.rb > line 182). You could: make a new method called “player1_move” ex:

def make_move(valid) @move = valid.rules.keys.sample end def player1_move @move puts ‘ ' end

Call this method after player 2 enters their move

e) Reflection: Overall, you make very good use of some advanced methods and do a smart job of managing your methods. I’ve learned how to call reusable methods in a more efficient way within different programs by studying your code. Thanks for that. Other reflections:

It appears that in tournament mode, the program only chooses a preset number of rounds. I think this is fine as well, but it should be displayed to the end user. Consider adding a method that displays the users current status in the game, whether their in a tournament or not. This could be accomplished in the Game class

StevenThiesfeld commented 9 years ago

There's alot of good suggestions here, thanks for the feedback! I just want to point out that my program technically has a tie breaker round for best_of even numbered matches where a tie is possible, it just isn't called a tie breaker round, it's just treated as say, a 7th round in a best of six match. After seeing your code though I started thinking about how I could work in some notification that the last match is a tiebreaker. Thanks again, I'll start integrating your suggestions!