Pull request for assignment 1 #29
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…d was the best way, the test knows too much about the implementation of the code, which makes it brittle... which way would you recommend for this?
… game itself, and the way of showing hidden cards is in the hand. that made most sense to me.
| @@ -13,8 +12,12 @@ def value | |||
| return @value | |||
| end | |||
|
|
|||
| def suit | |||
There was a problem hiding this comment.
here you're overriding the attr_reader :suit. I think most developers would expect the "suit" method to return the actual suit, so I'd probably go with another method.
In the Eagle, I typically recommend that instead of using suit as a symbol, you go ahead and make the suit a full class. There you'd set your own to_s method.
|
Most excellent job on this! I added one comment that you saw, but other than that I think it's spot on. regarding the let me know if you have questions on moving the suit to its own class! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hey Jesse, sorry for the delay, finally started working on the assignments.
Everything went... smoothly, I guess.
Just had some comments on assignment two, I had to modify some of the internal structure of the instance variables of game, so I used some sort of hack to do it.
How do you recommend doing this?
At first I tried game.player_hand = Hand.new but it didn't let me because there's not an accessor. However, it made sense not to have it, so I decided against it. The player shouldn't be able to modify it on the fly, right?
Thanks!