HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Piece class as part of implementation for the Tetris game

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
thepiecepartgameforimplementationclasstetris

Problem

Here is my Piece class as part of implementation for the Tetris game. If possible, I want my code to face the same level of scrutiny as actual production code.

Hereare some of the potential areas in which I feel my code could be improved, but due to my limited experience, was unable to do so. Please help me provide suggestions on, but not limited, to these areas.

-
I want to make "Piece next" variable "final", but don't know how it can be initialised in the constructor

-
I expect testGetNextPiece2 and testGetNextPiece3 in "PieceTest.java"
to pass, but both test methods failed, but I can't find anything wrong with my Piece class code nor the test methods.

-
Testing edge cases I have missed (I have focused solely on the "pyramid" piece, because I don't want to cluster this page with similar tests on different pieces).

Here is the link to the assignment requirements of which I also attached some screenshots below.

Requirements

class Piece

```
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Objects;
import java.util.StringTokenizer;

class Piece {

/**
* array of "TPoints" that make up the current instance of "Piece",
*/
private final TPoint[] body;
/**
* skirt is an int[] array with as many elements as the piece is wide,
* the skirt stores the lowest y value that appears in the body for each x value in the piece,
* the x values are the index into the array
*/
private final int[] skirt;
/**
* the width of piece after been placed on the lower left hand of the board
*/
private final int width;

/**
* the height of piece after been placed on the lower left hand of the board
*/
private final int height;
/**
* the resulting piece after rotating the current piece in counter clock direction
*/
private Piece next;

/**
* array that represent all possible pieces
*/
private static final Piece[] piec

Solution

Some initial thoughts:

  • Try to avoid numbered names. How is pyramid1 different to pyramid2? What does testGetNextPiece2 test that testGetNextPiece doesn't? The more descriptive your names are, the easier your code will to follow, change and debug.



  • getNextPiece returns the piece after it has been rotated counter clockwise. Why not call it getPieceRotatedCounterClockwise? It's more descriptive and you wouldn't need the comment to explain what it is...



  • myPoints.containsAll(otherPoints) what happens if myPoints contains all of otherPoints but otherPoints doesn't contain all of myPoints?



  • Don't leave commented out code. If you don't need it, delete it (use source control if you are worried about losing something). Leaving the comment there just creates noise. If a test isn't ready, then @Ignore ideally with a reason message to explain why it's disabled.

Context

StackExchange Code Review Q#134220, answer score: 3

Revisions (0)

No revisions yet.