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

attempted to write capture logic #13

Merged
merged 13 commits into from
Aug 5, 2017
Merged

attempted to write capture logic #13

merged 13 commits into from
Aug 5, 2017

Conversation

dennisgit42
Copy link
Contributor

I tried creating two pieces for the same game in rails console and having one piece move into the square held by another piece, but rails tells me that it cannot see my method move_to! I think it is problem with way my code is organized. Can you take a look and suggest how I might rearrange?

end
end
return nil
end

Choose a reason for hiding this comment

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

Clever! 😄

This code is iterating through every piece and checking the coordinates. Totally works.

To make this more efficient, can use ActiveRecord/SQL:

game.pieces.find_by(x_position: x, y_position: y)

Notice game.pieces. This ensures we pull pieces from the current game (it knows the current game because game is an association on Piece and we are currently inside of a Piece).

find_by will find the record or return nil.

@@ -1,6 +1,43 @@
class Piece < ApplicationRecord
belongs_to :game

def move_to!(new_x, new_y, white_turn)

Choose a reason for hiding this comment

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

Instead of passing white_turn.

If you call self.is_white?, you'll be able to see if the current piece that is being moved is white. Doing that will eliminate needing to pass in white_turn

@dennisgit42
Copy link
Contributor Author

Made the changes you suggested, but rubocop is still saying that

app/models/piece.rb:10:5: C: Use a guard clause instead of wrapping the code inside a conditional expression.
if (occupying_piece.is_white && !self.is_white?) || (!occupying_piece.is_white && self.is_white?)
^^

I don't understand what is wrong with using an if statement in this case. Can I just replace the if with unless and change each condition to its negation? (For example, unless (occupying_piece.is_white && self.is_white?) || (!occupying_piece.is_white && !self.is_white?) I looked at some pages regarding guard clauses, but can't see how it would be applied in my case. rubocop/rubocop#2903
http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/GuardClause

@dennisgit42
Copy link
Contributor Author

I fixed the issue with the guard clause and rubocop, just need to clean up the last few rubocop errors. Does anyone see an issue with the code I have so far?

Copy link
Contributor

@michellejanosi michellejanosi left a comment

Choose a reason for hiding this comment

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

Looks pretty great to me, Dennis. Have you made your updates on the Rubocop portion?

@dennisgit42
Copy link
Contributor Author

Okay, looks like all the tests pass.

return piece if piece.x_position == x && piece.y_position == y
end
nil
end

Choose a reason for hiding this comment

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

can you update this to use find_by (see previous note)

Added Melsen's strf format for date game was created line 7
@dennisgit42 dennisgit42 merged commit 683750e into master Aug 5, 2017
@dennisgit42 dennisgit42 deleted the dennis branch August 5, 2017 16:22
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.

3 participants