Tetris dueling #2
Conversation
ian-deans
force-pushed
the
tetris-dueling
branch
from
462a692
to
d8c5e04
Compare
|
|
||
| nextPositionIsRubble() { | ||
| const nextPos = this.fallingPiece.points() | ||
| .map( point => new Models.Point( point.row +1, point.col )) |
There was a problem hiding this comment.
Consider indenting this to increase readability.
| const nextPos = this.fallingPiece.points() | ||
| .map( point => new Models.Point( point.row +1, point.col )) | ||
| return nextPos | ||
| .some( point => this.rubble |
There was a problem hiding this comment.
This formatting was a little tough for me to parse - consider reformatting to make it a little bit clearer what's going on here...
| } | ||
| down() { | ||
| while ( | ||
| !this.fallingPieceIsOutOfBounds() && |
There was a problem hiding this comment.
Consider refactoring this into an intention revealing method:
while( this.pieceIsInValidPosition() ) {
// etc
}
pieceIsInValidPosition() {
return !this.fallingPieceIsOutOfBounds() && !this.fallingPieceOverlapsRubble()
}On a related note, if it increases the readability of this code, De Morgan's Law states that ! a && ! b is equivalent to !( a || b )
| @@ -97,7 +126,10 @@ export default class Game { | |||
| } | |||
|
|
|||
| fallingPieceOverlapsRubble() { | |||
| return this.fallingPiece.points().some( p => this.rubble.some( r => r.sameAs(p) ) ) | |||
| return this.fallingPiece.points() | |||
| .some( point => this.rubble | |||
There was a problem hiding this comment.
Same note as above, a little difficult to understand with this formatting.
| <div className='border' style={{ width: this.props.game.cols*25, | ||
| height: this.props.game.rows*25}}> | ||
| <div> | ||
| <NextPieceView piece={this.props.game.game1.nextPiece} className='next1'/> |
There was a problem hiding this comment.
This line should be outdented to preserve consistent formatting.
| state : | ||
| state = { 'game1': state.game1, 'game2': state.game2.down() } | ||
| case 'PAUSE': | ||
| pausedBoolean = ( pausedBoolean === false ) ? true : false |
|
|
||
| const ROTATIONS = ['N', 'E', 'S', 'W'] | ||
|
|
||
| class Rotation { |
There was a problem hiding this comment.
Consider re-organizing into its own module
|
|
||
| previous() { | ||
| return ROTATIONS[ ( this.index - 1 ) % 4 ] | ||
| } | ||
| } | ||
|
|
||
| export class Piece { |
There was a problem hiding this comment.
Consider re-organizing into its own module.
| this.shape = shape | ||
| this.offset = offset | ||
| this.rotation = 'N' | ||
| this.classString = shape.classString | ||
| } | ||
|
|
||
| points() { | ||
| return this.shape.pointsRotated( this.rotation ).map( ( point, ix ) => point.add( this.offset ) ); | ||
| return this.shape.pointsRotated( this.rotation ) | ||
| .map( ( point, ix ) => point.add( this.offset ) ); |
| this.shape = shape | ||
| this.offset = offset | ||
| this.rotation = 'N' | ||
| this.classString = shape.classString | ||
| } | ||
|
|
||
| points() { | ||
| return this.shape.pointsRotated( this.rotation ).map( ( point, ix ) => point.add( this.offset ) ); | ||
| return this.shape.pointsRotated( this.rotation ) |
There was a problem hiding this comment.
This feels like it may be another violation of the Law of Demeter (reaching through shape into another thing to perform an operation)...
No description provided.