Piece class as part of implementation for the Tetris game

Question

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.

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

2. 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.

3. 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

enter image description here

enter image description here

enter image description here

enter image description here

enter image description here

enter image description here

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[] pieces;
 static {
 pieces = Piece.getPieces();
 }
 /**
 * construct a new piece object from a existing array of TPoint, 
 * @param points - array of TPoint that together make up the body of a Piece object
 */
 Piece(TPoint[] points) {
 body = new TPoint[points.length];
 for (int i = 0; i < points.length; i++) {
 body[i] = new TPoint(points[i]);
 }
 //calculate "width" and "height" of the piece using the "body" 
 int xMax = 0;
 int yMax = 0;
 for (TPoint point : body) {
 if (xMax < point.x) {
 xMax = point.x;
 }
 if (yMax < point.y) {
 yMax = point.y;
 }
 }
 //because coordinate of board starts from (x, y) = (0, 0) from the lower left corner,
 //therefore in order to calculate actual "width" and "height" of the piece,
 //have to add 1 to the x coordinate of the piece at the far right to get the width,
 //and add 1 to the y coordinate of the piece at the top to get the height
 width = xMax + 1;
 height = yMax + 1;
 skirt = calculateSkirt();
 }
 /**
 * convenient initializer, initialize a new Piece from a existing piece
 * @param piece 
 */
 Piece(Piece piece) {
 this(piece.body);
 }
 /**
 * convenient initializer, initialize a new Piece from a string that represent 
 * x and y coordinate of TPoints that make up the body of a piece 
 * @param points a string that contain x and y coordinates of TPoints in pair, 
 * e.g. "0 0 0 1" represents two TPoints at coordinate (0, 0) and (0, 1) 
 */
 Piece(String points) {
 this(parsePoints(points));
 }
 /**
 * public getter for the next piece after rotation 
 * @return the resulting piece after the current piece rotate 90 degrees in counter clock direction 
 */
 public Piece getNextPiece() {
 return next;
 }
 public int[] getSkirt() {
 return skirt;
 }
 public int getHeight() {
 return height;
 }
 public int getWidth() {
 return width;
 }
 /**
 * goes through each TPoint that make up the current piece, 
 * for a given x coordinate,
 * find the piece located on this x coordinate that has the lowest y coordinate.
 * 
 * x coordinate of each TPoint can be thought of as index number in "int[] skirt"
 * @return an array that contain the lowest y coordinate in each x coordinate 
 */
 private int[] calculateSkirt() {
 int[] pieceSkirt = new int[width];
 Arrays.fill(pieceSkirt, height - 1);
 for (TPoint point : body) {
 if (point.y < pieceSkirt[point.x]) {
 pieceSkirt[point.x] = point.y;
 }
 }
 return pieceSkirt;
 }
 // String constants for the standard 7 tetris pieces
 static final String STICK_STRING = "0 0 0 1 0 2 0 3";
 static final String L1_STRING = "0 0 0 1 0 2 1 0";
 static final String L2_STRING = "0 0 1 0 1 1 1 2";
 static final String S1_STRING = "0 0 1 0 1 1 2 1";
 static final String S2_STRING = "0 1 1 1 1 0 2 0";
 static final String SQUARE_STRING = "0 0 0 1 1 0 1 1";
 static final String PYRAMID_STRING = "0 0 1 0 1 1 2 0";
 // Indexes for the standard 7 pieces in the pieces array
 static final int STICK = 0;
 static final int L1 = 1;
 static final int L2 = 2;
 static final int S1 = 3;
 static final int S2 = 4;
 static final int SQUARE = 5;
 static final int PYRAMID = 6;
 /**
 * calculate all possible pieces in all possible orientation
 * @return array of all possible pieces in its original position, 
 * each piece contain a variable "next", which is a pointer to the piece 
 * after the current piece rotate 90 degrees in counter clock direction 
 */
 static Piece[] getPieces() {
 //lazy evaluation -- create static array when needed
 if (Piece.pieces == null) {
 Piece[] piecesCreated = new Piece[]{
 makeFastRotation(new Piece(STICK_STRING)),
 makeFastRotation(new Piece(L1_STRING)),
 makeFastRotation(new Piece(L2_STRING)),
 makeFastRotation(new Piece(S1_STRING)),
 makeFastRotation(new Piece(S2_STRING)),
 makeFastRotation(new Piece(SQUARE_STRING)),
 makeFastRotation(new Piece(PYRAMID_STRING))
 };
 return piecesCreated;
 } else {
 return Piece.pieces;
 }
 }
 /**
 * calculate all possible position of a piece
 * @param passedInPiece the piece in its original position
 * @return the passed in original piece, but now this piece has a variable "next",
 * that is initialized and is pointing to the piece after rotation was completed,
 * this forms a circle of points (i.e. variable "next") which eventually points back to the original piece.
 */
 private static Piece makeFastRotation(Piece passedInPiece) {
 Piece currentPiece = new Piece(passedInPiece);
 Piece myRootPiece = currentPiece;
 while (true) {
 Piece nextRotation = currentPiece.computeNextRotation();
 if (nextRotation.equals(myRootPiece)) {
 currentPiece.next = myRootPiece;
 break;
 }
 currentPiece.next = nextRotation;
 currentPiece = currentPiece.next;
 }
 return myRootPiece;
 }
 /**
 * calculate the position of next piece after the current piece rotate 90 degrees 
 * in counter clock direction
 * @return the position of next piece after rotation 
 */
 Piece computeNextRotation() {
 TPoint[] rotatedBody = new TPoint[body.length];
 for (int i = 0; i < body.length; i++) {
 rotatedBody[i] = new TPoint(height - 1 - body[i].y, body[i].x);
 }
 Piece computedNext = new Piece(rotatedBody);
 return computedNext;
 }
 @Override
 public boolean equals(Object obj) {
 //standard equals() technique 1
 if (this == obj) {
 return true;
 }
 //standard equals() technique 2
 if (!(obj instanceof Piece)) {
 return false;
 }
 Piece other = (Piece) obj;
 if (other.body.length != body.length) {
 return false;
 }
 List<TPoint> myPoints = Arrays.asList(body);
 List<TPoint> otherPoints = Arrays.asList(other.body);
 return myPoints.containsAll(otherPoints);
 }
 @Override
 public int hashCode() {
 int hash = 5;
 hash = 37 * hash + Arrays.deepHashCode(this.body);
 hash = 37 * hash + Arrays.hashCode(this.skirt);
 hash = 37 * hash + this.width;
 hash = 37 * hash + this.height;
 hash = 37 * hash + Objects.hashCode(this.next);
 return hash;
 }
 /**
 * convert a string that represents the x and y coordinate of an array of TPoints 
 * to an actual array of TPoints
 * @param string a string that represents the x and y coordinate of an array of TPoints
 * @return array of TPoints that represents the body location of a piece 
 */
 private static TPoint[] parsePoints(String string) {
 List<TPoint> points = new ArrayList<TPoint>();
 StringTokenizer tok = new StringTokenizer(string);
 try {
 while (tok.hasMoreTokens()) {
 int x = Integer.parseInt(tok.nextToken());
 int y = Integer.parseInt(tok.nextToken());
 points.add(new TPoint(x, y));
 }
 } catch (NumberFormatException e) {
 throw new RuntimeException("Could not parse x,y string:" + string);
 }
 // Make an array out of the collection
 TPoint[] array = points.toArray(new TPoint[0]);
 return array;
 }
}

Test class:

import org.junit.Before;
import org.junit.Test;
import static org.junit.Assert.*;
/**
 *
 * @author justin
 */
public class PieceTest {
 private Piece pyramid1, pyramid2, pyramid3, pyramid4;
 private Piece square1, square2, square3;
 private Piece stick1, stick2, stick3;
 private Piece L21, L22, L24;
 private Piece s, sRotated;
 private Piece[] pieces;
 @Before
 public void setUp() {
 pyramid1 = new Piece(Piece.PYRAMID_STRING);
 pyramid2 = pyramid1.computeNextRotation();
 pyramid3 = pyramid2.computeNextRotation();
 pyramid4 = pyramid3.computeNextRotation();
 square1 = new Piece(Piece.SQUARE_STRING);
 square2 = square1.computeNextRotation();
 square3 = square2.computeNextRotation();
 stick1 = new Piece(Piece.STICK_STRING);
 stick2 = stick1.computeNextRotation();
 stick3 = stick2.computeNextRotation();
 L21 = new Piece(Piece.L2_STRING);
 L22 = L21.computeNextRotation();
 L24 = L22.computeNextRotation().computeNextRotation();
 s = new Piece(Piece.S1_STRING);
 sRotated = s.computeNextRotation();
 pieces = Piece.getPieces();
 }
 /**
 * Test of getNextPiece method, of class Piece.
 */
 @Test
 public void testGetNextPiece() {
 System.out.println("getNextPiece");
 assertTrue(pyramid1.equals(pieces[Piece.PYRAMID]));
 assertTrue(pyramid2.equals(pieces[Piece.PYRAMID].getNextPiece()));
 }
 @Test
 public void testGetNextPiece2(){
 System.out.println("getNextPiece2");
 assertTrue(pyramid2.equals(pyramid1.getNextPiece()));
 }
 @Test
 public void testGetNextPiece3(){
 System.out.println("getNextPiece3");
 assertEquals(pyramid2, pyramid1.getNextPiece());
 }
 /**
 * Test of getSkirt method, of class Piece.
 */
 @Test
 public void testGetSkirt() {
 System.out.println("getSkirt");
 assertArrayEquals(new int[]{0, 1}, pyramid4.getSkirt());
 }
 /**
 * Test of getHeight method, of class Piece.
 */
 @Test
 public void testGetHeight() {
 System.out.println("getHeight");
 assertEquals(2, pyramid1.getHeight());
 }
 /**
 * Test of getWidth method, of class Piece.
 */
 @Test
 public void testGetWidth() {
 System.out.println("getWidth");
 assertEquals(3, pyramid1.getWidth());
 }
 /**
 * Test of equals method, of class Piece.
 */
 @Test
 public void testEquals() {
 System.out.println("equals");
 boolean expResult = true;
 boolean result = pyramid1.equals(pyramid4.computeNextRotation());
 assertEquals(expResult, result);
 }
 @Test
 public void testEquals2(){
 System.out.println("equals2");
 boolean expectedResult = true;
 Piece rootPiece = pieces[Piece.PYRAMID].getNextPiece().getNextPiece().getNextPiece().getNextPiece();
 boolean result = pyramid1.equals(rootPiece);
 assertEquals(expectedResult, result);
 }
// /**
// * Test of hashCode method, of class Piece.
// */
// @Test
// public void testHashCode() {
// System.out.println("hashCode");
// Piece instance = null;
// int expResult = 0;
// int result = instance.hashCode();
// assertEquals(expResult, result);
// // TODO review the generated test code and remove the default call to fail.
// fail("The test case is a prototype.");
// }
 /**
 * Test of getPieces method, of class Piece.
 */
 @Test
 public void testGetPieces() {
 System.out.println("getPieces");
 Piece[] expResult = new Piece[]{
 new Piece(Piece.STICK_STRING),
 new Piece(Piece.L1_STRING),
 new Piece(Piece.L2_STRING),
 new Piece(Piece.S1_STRING),
 new Piece(Piece.S2_STRING),
 new Piece(Piece.SQUARE_STRING),
 new Piece(Piece.PYRAMID_STRING)
 };
 Piece[] result = Piece.getPieces();
 assertArrayEquals(expResult, result);
 }
 /**
 * Test of computeNextRotation method, of class Piece.
 */
 @Test
 public void testComputeNextRotation() {
 System.out.println("computeNextRotation");
 TPoint[] points = new TPoint[]{
 new TPoint(0, 1),
 new TPoint(1, 0),
 new TPoint(1, 1),
 new TPoint(1, 2)
 };
 Piece expResult = new Piece(points);
 Piece result = pyramid1.computeNextRotation();
 assertEquals(expResult, result);
 }
}

• Gaurava Agarwal: I have written this game during Job selection process of one mobile game company. Hopefully I Will provide you something.
• Thor: @GauravaAgarwal that would be wonderful. Any assistant would be greatly appreciated!
• Grim: Dejaveu, i wrote tetris when i was 17 in turbo pascal.
• Thor: @PeterRader could you please have a look at my code? I'm a relatively newbie at programming, really want to improve my code quality. Thanks in advance for any help!

Answer 1

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.