0
\$\begingroup\$

I've began to read the book Clean Code by Robert Martin, I had a strong desire to learn how to write code that's easy for others to understand. Please criticize my code.

PuzzlesView.java

// Using of PuzzlesView:
// dimension = new Dimension(rows, columns)
// puzzlesView.set(bitmap, dimension);
// after the setting the puzzlesView will scale the bitmap to its size,
// divide the scaled bitmap by puzzles and show mixed puzzles.
// if you want to mix the puzzles again, then use the call mix()
// puzzlesView.mix();
public class PuzzlesView extends View {
    private final int LATTICE_WIDTH = 2;
    private final float ALLOWABLE_ERROR = 0.4f;

    private Dimension dim;
    private Bitmap fullImage;
    private Matrix<Bitmap> puzzles;
    private Mixer mixer = new Mixer();
    private Collection<OnGameFinishedListener> onGameFinishedListeners =
                                new ArrayList<OnGameFinishedListener>();
    private GameArbitrator arbitrator;
    private Size puzzleSize;
    private Point lastTouchedPoint;
    private Point draggedLeftUpper;
    private Matrix.Position draggedPosition;
    private Matrix.Position nearestPosition;
    private Canvas canvas;


    public PuzzlesView(Context context) {
        super(context);
    }

    public PuzzlesView(Context context, AttributeSet attrs) {
        super(context, attrs);
    }

    public PuzzlesView(Context context, AttributeSet attrs, int defStyle) {
        super(context, attrs, defStyle);
    }

    public void addOnGameFinishedListener(OnGameFinishedListener listener) {
        onGameFinishedListeners.add(listener);
    }

    public void set(Bitmap bitmap, Dimension dim) {
        if (imageWasSet()) {
            releasePreviousImageResources();
        }
        setDimension(dim);
        setBitmap(bitmap);
    }

    private boolean imageWasSet() {
        return (fullImage != null);
    }

    private void releasePreviousImageResources() {
        fullImage.recycle();
        puzzles.forEach(new Matrix.OnEachHandler<Bitmap>() {
            @Override
            public void handle(Matrix<Bitmap> matrix, Position pos) {
                Bitmap bitmap = matrix.get(pos);
                bitmap.recycle();
                matrix.set(pos, null);
            }
        });
    }

    private void setDimension(Dimension dim) {
        this.dim = dim;
        puzzles = new Matrix<Bitmap>(dim);
    }

    private void setBitmap(Bitmap bitmap) {
        draggingStopped();
        calculatePuzzleSize();
        fullImage = scaledToFullSize(bitmap);
        cutIntoPuzzles();
        arbitrator = new GameArbitrator(puzzles);
        mix();
    }

    private void draggingStopped() {
        draggedPosition = null;
    }

    private void calculatePuzzleSize() {
        int width = getWidth() - LATTICE_WIDTH * (dim.columns - 1);
        int height = getHeight() - LATTICE_WIDTH * (dim.rows - 1);
        puzzleSize = new Size(width / dim.columns, height / dim.rows);
    }

    private Bitmap scaledToFullSize(Bitmap bitmap) {
        int fullImageWidth = puzzleSize.width * dim.columns;
        int fullImageHeight = puzzleSize.height * dim.rows;
        return Bitmap.createScaledBitmap(bitmap, fullImageWidth, fullImageHeight, true);
    }

    private void cutIntoPuzzles() {
        puzzles.forEach(new Matrix.OnEachHandler<Bitmap>() {
            @Override
            public void handle(Matrix<Bitmap> matrix, Position pos) {
                matrix.set(pos, puzzleByPosition(pos));
            }
        });
    }

    private Bitmap puzzleByPosition(Matrix.Position pos) {
        int x = pos.column * puzzleSize.width;
        int y = pos.row * puzzleSize.height;
        return Bitmap.createBitmap(fullImage, x, y, puzzleSize.width, puzzleSize.height);
    }

    public void mix() {
        mixer.mix(puzzles);
        invalidate();
    }

    @Override
    protected void onDraw(Canvas canvas) {
        if (imageWasSet()) {
            this.canvas = canvas;
            drawLattice();
            drawPuzzles();
        }
    }

    private void drawLattice() {
        Point rightDownPoint = puzzlesRightDownPoint();
        Paint latticePaint = preparePaintForLattice();
        drawVerticalLatticeLines(rightDownPoint, latticePaint);
        drawHorizontalLatticeLines(rightDownPoint, latticePaint);
    }

    private Point puzzlesRightDownPoint() {
        Point point = leftUpperOfPuzzle(new Matrix.Position(dim.rows, dim.columns));
        return new Point(point.x - LATTICE_WIDTH, point.y - LATTICE_WIDTH);
    }

    private Point leftUpperOfPuzzle(Matrix.Position pos) {
        int x = (puzzleSize.width + LATTICE_WIDTH) * pos.column;
        int y = (puzzleSize.height + LATTICE_WIDTH) * pos.row;
        return new Point(x, y);
    }

    private Paint preparePaintForLattice() {
        Paint paint = new Paint();
        paint.setColor(ResourceReader.colorById(R.color.lattice_color));
        paint.setStrokeWidth(LATTICE_WIDTH);
        return paint;
    }

    private void drawVerticalLatticeLines(Point rightDownPoint, Paint latticePaint) {
        for (int column = 1; column < dim.columns; ++column) {
            Point startPoint = latticeLineStartPoint(0, column);
            canvas.drawLine(startPoint.x, 0, startPoint.x, rightDownPoint.y, latticePaint);
        }
    }

    private void drawHorizontalLatticeLines(Point rightDownPoint, Paint latticePaint) {
        for (int row = 1; row < dim.rows; ++row) {
            Point startPoint = latticeLineStartPoint(row, 0);
            canvas.drawLine(0, startPoint.y, rightDownPoint.x, startPoint.y, latticePaint);
        }
    }

    private Point latticeLineStartPoint(int row, int column) {
        Point point = leftUpperOfPuzzle(new Position(row, column));
        return new Point(point.x - 1, point.y - 1);
    }

    private void drawPuzzles() {
        puzzles.forEach(new Matrix.OnEachHandler<Bitmap>() {
            @Override
            public void handle(Matrix<Bitmap> matrix, Position pos) {
                if (!existDraggedPuzzle() || !pos.equals(draggedPosition)) {
                    Point leftUpper = leftUpperOfPuzzle(pos);
                    Paint paint = paintForPosition(pos);
                    Bitmap puzzle = puzzles.get(pos);
                    canvas.drawBitmap(puzzle, leftUpper.x, leftUpper.y, paint);
                }
            }
        });
        if (existDraggedPuzzle()) {
            Bitmap draggedPuzzle = puzzles.get(draggedPosition);
            canvas.drawBitmap(draggedPuzzle, draggedLeftUpper.x, draggedLeftUpper.y, null);
        }
    }

    private boolean existDraggedPuzzle() {
        return draggedPosition != null;
    }

    private Paint paintForPosition(Position pos) {
        return positionOfCommutablePuzzle(pos)
                    ? commutablePuzzlePaint()
                    : null;
    }

    private boolean positionOfCommutablePuzzle(Position pos) {
        return draggedAndNearestCanBeSwapped() && pos.equals(nearestPosition);
    }

    private boolean draggedAndNearestCanBeSwapped() {
        if (nearestPosition == null) {
            return false;
        }
        Point nearestLeftUpper = leftUpperOfPuzzle(nearestPosition);
        int dx = Math.abs(nearestLeftUpper.x - draggedLeftUpper.x);
        int dy = Math.abs(nearestLeftUpper.y - draggedLeftUpper.y);
        return allowableOffset(dx, dy);
    }

    private boolean allowableOffset(int dx, int dy) {
        boolean dxAllowable = (dx <= ALLOWABLE_ERROR * puzzleSize.width);
        boolean dyAllowable = (dy <= ALLOWABLE_ERROR * puzzleSize.height);
        return dxAllowable && dyAllowable;
    }

    private Paint commutablePuzzlePaint() {
        Paint paint = new Paint();
        int color = ResourceReader.colorById(R.color.commutable_puzzle_color);
        ColorFilter filter = new LightingColorFilter(color, 1);
        paint.setColorFilter(filter);
        return paint;
    }

    @Override
    public boolean onTouchEvent(MotionEvent event) {
        if (!imageWasSet()) {
            return false;
        }
        switch (event.getAction()) {
        case MotionEvent.ACTION_DOWN:
            onDownTouch(eventPoint(event));
            break;

        case MotionEvent.ACTION_MOVE:
            onMoveTouch(eventPoint(event));
            break;

        case MotionEvent.ACTION_UP:
            onUpTouch(eventPoint(event));
            break;
        }
        return true;
    }

    private Point eventPoint(MotionEvent event) {
        int x = (int) event.getX();
        int y = (int) event.getY();
        return new Point(x, y);
    }

    private void onDownTouch(Point pt) {
        Matrix.Position pos = positionByPoint(pt);
        if (insideGameBoard(pos) && insidePuzzle(pt)) {
            lastTouchedPoint = pt;
            draggedPosition = pos;
            draggedLeftUpper = leftUpperOfPuzzle(pos);
        }
    }

    private Matrix.Position positionByPoint(Point pt) {
        int column = pt.x / (puzzleSize.width + LATTICE_WIDTH);
        int row = pt.y / (puzzleSize.height + LATTICE_WIDTH);
        return new Matrix.Position(row, column);
    }

    private boolean insideGameBoard(Position pos) {
        return pos.row >= 0 && pos.row < dim.rows &&
               pos.column >= 0 && pos.column < dim.columns;
    }

    private boolean insidePuzzle(Point pt) {
        return ((pt.x % (puzzleSize.width + LATTICE_WIDTH)) < puzzleSize.width) &&
               ((pt.y % (puzzleSize.height + LATTICE_WIDTH)) < puzzleSize.height);
    }

    private void onMoveTouch(Point pt) {
        if (existDraggedPuzzle()) {
            int dx = pt.x - lastTouchedPoint.x;
            int dy = pt.y - lastTouchedPoint.y;
            draggedLeftUpper = new Point(draggedLeftUpper.x + dx, draggedLeftUpper.y + dy);
            lastTouchedPoint = pt;
            calculateNearestForDragged();
            invalidate();
        }
    }

    private void calculateNearestForDragged() {
        if (draggedLeftUpper.x >= 0 && draggedLeftUpper.y >= 0) {
            Matrix.Position pos1 = positionByPoint(draggedLeftUpper);
            Matrix.Position pos2 = new Matrix.Position(pos1.row, pos1.column + 1);
            Matrix.Position pos3 = new Matrix.Position(pos1.row + 1, pos1.column);
            Matrix.Position pos4 = new Matrix.Position(pos1.row + 1, pos1.column + 1);
            calculateNearestForDragged(pos1, pos2, pos3, pos4);
        }
    }

    private void calculateNearestForDragged(Matrix.Position... positions) {
        for (Matrix.Position each : positions) {
            if (insideGameBoard(each)) {
                checkCloseness(each);
            }
        }
    }

    private void checkCloseness(Position pos) {
        if (nearestPosition == null || !insideGameBoard(nearestPosition)) {
            nearestPosition = pos;
        } else if (distanceFromDraggedTo(pos) < distanceFromDraggedTo(nearestPosition)) {
            nearestPosition = pos;
        }
    }

    private int distanceFromDraggedTo(Position pos) {
        Point leftUpper = leftUpperOfPuzzle(pos);
        int dx = leftUpper.x - draggedLeftUpper.x;
        int dy = leftUpper.y - draggedLeftUpper.y;
        return dx * dx + dy * dy;
    }

    private void onUpTouch(Point pt) {
        if (!existDraggedPuzzle()) {
            return;
        }
        if (draggedAndNearestCanBeSwapped()) {
            puzzles.swap(nearestPosition, draggedPosition);
        }
        nearestPosition = null;
        draggingStopped();
        invalidate();
        if (arbitrator.gameFinished(puzzles)) {
            notifyThatGameFinished();
        }
    }

    private void notifyThatGameFinished() {
        for (OnGameFinishedListener each : onGameFinishedListeners) {
            each.onGameFinished();
        }
    }
}

ResourceReader.java

public class ResourceReader {
    private static Resources resources;

    public static void init(Resources res) {
        resources = res;
    }

    public static int colorById(int id) {
        return resources.getColor(id);
    }
}

Mixer.java

public class Mixer {
    private Random random;

    public void mix(Matrix<Bitmap> puzzles) {
        random = new Random(System.nanoTime());

        int numberOfSwaps = numberOfSwaps(puzzles);
        for (int i = 0; i < numberOfSwaps; ++i) {
            puzzles.swap(generateRandomPosition(puzzles), generateRandomPosition(puzzles));
        }
    }

    private int numberOfSwaps(Matrix<Bitmap> puzzles) {
        return puzzles.rows * puzzles.columns * 2;
    }

    private Position generateRandomPosition(Matrix<Bitmap> puzzles) {
        int row = random.nextInt(puzzles.rows);
        int column = random.nextInt(puzzles.columns);
        return new Position(row, column);
    }
}

GameArbitrator.java

public class GameArbitrator {
    private Matrix<Bitmap> startPuzzles;

    public GameArbitrator(Matrix<Bitmap> puzzles) {
        startPuzzles = new Matrix<Bitmap>(puzzles);
    }

    public boolean gameFinished(Matrix<Bitmap> puzzles) {
        for (int row = 0; row < puzzles.rows; ++row) {
            for (int column = 0; column < puzzles.columns; ++column) {
                if (startPuzzles.get(row, column) != puzzles.get(row, column)) {
                    return false;
                }
            }
        }
        return true;
    }
}

Dimension.java

public class Dimension {
    public final int rows;
    public final int columns;

    public Dimension(int rows, int columns) {
        if (rows <= 0 || columns <= 0) {
            throw new IllegalArgumentException(
                    "Dimension constructor: rows and columns must be positive");
        }
        this.rows = rows;
        this.columns = columns;
    }

}

TestDimension.java

public class TestDimesion {
    @Test
    public void testCreation() {
        final int rows = 1;
        final int columns = 2;
        Dimension dim = new Dimension(rows, columns);
        assertEquals(rows, dim.rows);
        assertEquals(columns, dim.columns);
    }

    @Test(expected=IllegalArgumentException.class)
    public void testRowsIsZero() {
        new Dimension(0, 1);
    }

    @Test(expected=IllegalArgumentException.class)
    public void testRowsIsNegative() {
        new Dimension(-1, 1);
    }

    @Test(expected=IllegalArgumentException.class)
    public void testColumnsIsZero() {
        new Dimension(1, 0);
    }

    @Test(expected=IllegalArgumentException.class)
    public void testColumnsIsNegative() {
        new Dimension(1, -1);
    }
}

Size.java

public class Size {
    public final int width;
    public final int height;

    public Size(int width, int height) {
        if (width < 0 || height < 0) {
            throw new IllegalArgumentException();
        }
        this.width = width;
        this.height = height;
    }
}

TestSize.java

public class TestSize {
    @Test
    public void testCreation() {
        Size size = new Size(1, 2);
        assertEquals(1, size.width);
        assertEquals(2, size.height);
    }

    @Test(expected=IllegalArgumentException.class)
    public void testIllegalSize() {
        new Size(1, -2);
    }
}

Matrix.java

public class Matrix<T> {

    public interface OnEachHandler<T> {
        void handle(Matrix<T> matrix, Position pos);
    }

    public static class Position {
        public final int row;
        public final int column;

        public Position(int row, int column) {
            if (row < 0 || column < 0) {
                throw new IllegalArgumentException();
            }
            this.row = row;
            this.column = column;
        }

        @Override
        public boolean equals(Object obj) {
            if ((obj == null) || !(obj instanceof Matrix.Position)) {
                return false;
            }
            Matrix.Position other = (Matrix.Position) obj;
            return (this.row == other.row) && (this.column == other.column);
        }
    }

    public final int rows;
    public final int columns;
    private T[] values;

    @SuppressWarnings("unchecked")
    public Matrix(int rows, int columns) {
        if (rows <= 0 || columns <= 0) {
            throw new IllegalArgumentException();
        }
        this.rows = rows;
        this.columns = columns;
        values = (T[]) new Object[rows * columns];
    }

    public Matrix(Dimension dim) {
        this(dim.rows, dim.columns);
    }

    public Matrix(Matrix<T> other) {
        this(other.rows, other.columns);
        other.forEach(new OnEachHandler<T>() {
            @Override
            public void handle(Matrix<T> matrix, Position pos) {
                Matrix.this.set(pos, matrix.get(pos));
            }
        });
    }

    public void set(Position pos, T value) {
        set(pos.row, pos.column, value);
    }

    public void set(int row, int column, T value) {
        checkIndexes(row, column);
        values[index(row, column)] = value;
    }

    private void checkIndexes(int row, int column) {
        if (row < 0 || column < 0) {
            throw new IllegalArgumentException();
        }
        if (row >= rows || column >= columns) {
            throw new IndexOutOfBoundsException();
        }
    }

    private int index(int row, int column) {
        return row * columns + column;
    }

    public T get(Position pos) {
        return get(pos.row, pos.column);
    }

    public T get(int row, int column) {
        checkIndexes(row, column);
        return values[index(row, column)];
    }

    @Override
    public boolean equals(Object obj) {
        if (!(obj instanceof Matrix<?>)) {
            return false;
        }
        @SuppressWarnings("unchecked")
        Matrix<T> other = (Matrix<T>) obj;
        if (other.rows != rows || other.columns != columns) {
            return false;
        }
        for (int row = 0; row < rows; ++row) {
            for (int column = 0; column < columns; ++column) {
                T elementOfThis = get(row, column);
                T elementOfOther = other.get(row, column);
                if (!elementOfThis.equals(elementOfOther)) {
                    return false;
                }
            }
        }
        return true;
    }

    @Override
    public int hashCode() {
        int hashCode1 = get(0, 0).hashCode();
        int hashCode2 = get(rows - 1, columns - 1).hashCode();
        int hashCode = hashCode1 * hashCode2;
        hashCode += Math.max(hashCode1, hashCode2) / Math.min(hashCode1, hashCode2);
        return hashCode;
    }

    public void swap(Position pos1, Position pos2) {
        T temp = get(pos1);
        set(pos1, get(pos2));
        set(pos2, temp);
    }

    public void forEach(OnEachHandler<T> onEachHandler) {
        for (int row = 0; row < this.rows; ++row) {
            for (int column = 0; column < this.columns; ++column) {
                onEachHandler.handle(this, new Position(row, column));
            }
        }
    }
}

TestMatrix.java

public class TestMatrix {
    private static final Integer VALUE = 5;
    private static final int ROWS = 10;
    private static final int COLUMNS = 20;
    private Matrix<Integer> matrix;

    @Before
    public void setUp() {
        this.matrix = new Matrix<Integer>(ROWS, COLUMNS);
    }

    @Test
    public void testMatrix() {
        fillMatrix(matrix);
        matrix.forEach(new Matrix.OnEachHandler<Integer>() {
            @Override
            public void handle(Matrix<Integer> matrix, Matrix.Position pos) {
                assertEquals(elementForPosition(pos), matrix.get(pos));
            }
        });
    }

    private void fillMatrix(Matrix<Integer> matrix) {
        matrix.forEach(new Matrix.OnEachHandler<Integer>() {
            @Override
            public void handle(Matrix<Integer> matrix, Matrix.Position pos) {
                matrix.set(pos, elementForPosition(pos));
            }
        });
    }

    private Integer elementForPosition(Matrix.Position pos) {
        return pos.row * pos.column;
    }

    @Test
    public void testMatrixCopyConstructor() {
        fillMatrix(matrix);
        Matrix<Integer> copy = new Matrix<Integer>(matrix);
        assertEquals(matrix.rows, copy.rows);
        assertEquals(matrix.columns, copy.columns);
        copy.forEach(new Matrix.OnEachHandler<Integer>() {
            @Override
            public void handle(Matrix<Integer> matrix, Matrix.Position pos) {
                Integer expected = TestMatrix.this.matrix.get(pos);
                assertEquals(expected, matrix.get(pos));
            }
        });
    }

    @Test
    public void testInitValueIsNull() {
        assertNull(matrix.get(0, 0));
    }

    @Test
    public void testDimensionOfMatrix() {
        assertEquals(ROWS, matrix.rows);
        assertEquals(COLUMNS, matrix.columns);
    }

    @Test(expected=IllegalArgumentException.class)
    public void testConstructorIllegalArguments() {
        new Matrix<Integer>(1, -1);
    }

    @Test(expected=IllegalArgumentException.class)
    public void testSetIllegalArguments() {
        matrix.set(1, -1, VALUE);
    }

    @Test(expected=IndexOutOfBoundsException.class)
    public void testSetOutOfBoundsArguments() {
        matrix.set(0, COLUMNS + 1, VALUE);
    }

    @Test(expected=IllegalArgumentException.class)
    public void testGetIllegalArguments() {
        matrix.get(1, -1);
    }

    @Test(expected=IndexOutOfBoundsException.class)
    public void testGetOutOfBoundsArguments() {
        matrix.get(ROWS + 1, 3);
    }

    @Test
    public void testSwap() {
        Matrix<Integer> matrix = new Matrix<Integer>(2, 2);
        Matrix.Position pos1 = new Matrix.Position(0, 0);
        Matrix.Position pos2 = new Matrix.Position(1, 1);
        Integer val1 = 1;
        Integer val2 = 2;
        matrix.set(pos1, val2);
        matrix.set(pos2, val1);
        matrix.swap(pos1, pos2);
        assertEquals(val2, matrix.get(pos2));
        assertEquals(val1, matrix.get(pos1));
    }

    @Test(expected=IllegalArgumentException.class)
    public void testSwapIllegalArguments() {
        Matrix.Position pos1 = new Matrix.Position(0, 0);
        Matrix.Position pos2 = new Matrix.Position(0, -1);
        matrix.swap(pos1, pos2);
    }

    @Test(expected=IndexOutOfBoundsException.class)
    public void testSwapPositionOutOfBounds() {
        Matrix.Position pos1 = new Matrix.Position(0, COLUMNS + 1);
        Matrix.Position pos2 = new Matrix.Position(0, 0);
        matrix.swap(pos1, pos2);
    }

    @Test
    public void testEquals() {
        Matrix<Integer> matrix1 = new Matrix<Integer>(1, 2);
        matrix1.set(0, 0, VALUE);
        matrix1.set(0, 1, VALUE);
        Matrix<Integer> matrix2 = new Matrix<Integer>(1, 2);
        matrix2.set(0, 0, VALUE);
        matrix2.set(0, 1, VALUE);
        assertTrue(matrix1.equals(matrix2));
        matrix2.set(0, 0, VALUE + 1);
        assertFalse(matrix1.equals(matrix2));
    }

    @Test
    public void testForEach() {
        Matrix<Integer> counts = new Matrix<Integer>(matrix.rows, matrix.columns);
        for (int row = 0; row < counts.rows; ++row) {
            for (int column = 0; column < counts.columns; ++column) {
                counts.set(row, column, 0);
            }
        }
        counts.forEach(new Matrix.OnEachHandler<Integer>() {
            @Override
            public void handle(Matrix<Integer> matrix, Matrix.Position pos) {
                matrix.set(pos, matrix.get(pos) + 1);
            }
        });
        for (int row = 0; row < counts.rows; ++row) {
            for (int column = 0; column < counts.columns; ++column) {
                assertTrue(counts.get(row, column) == 1);
            }
        }
    }

    @Test
    public void testForEachOrder_LeftToRight_UpToDown() {
        Matrix<Boolean> flags = new Matrix<Boolean>(5, 7);
        flags.forEach(new Matrix.OnEachHandler<Boolean>() {
            @Override
            public void handle(Matrix<Boolean> matrix, Position pos) {
                matrix.set(pos, false);
            }
        });
        flags.forEach(new Matrix.OnEachHandler<Boolean>() {
            @Override
            public void handle(Matrix<Boolean> matrix, Position pos) {
                assertPreviousElementByPassed(matrix, pos);
                assertNextElementNotByPassed(matrix, pos);
                matrix.set(pos, true);
            }
        });
    }

    private void assertPreviousElementByPassed(Matrix<Boolean> matrix, Position pos) {
        if (!pos.equals(new Matrix.Position(0, 0))) {
            Matrix.Position positionBefore = positionBefore(matrix.rows, matrix.columns, pos);
            assertTrue(matrix.get(positionBefore));
        }
    }

    private void assertNextElementNotByPassed(Matrix<Boolean> matrix, Position pos) {
        Matrix.Position lastPos = new Matrix.Position(matrix.rows - 1, matrix.columns - 1);
        if (!pos.equals(lastPos)) {
            Matrix.Position positionAfter = positionAfter(matrix.rows, matrix.columns, pos);
            assertFalse(matrix.get(positionAfter));
        }
    }

    private Matrix.Position positionBefore(int rows, int columns, Position pos) {
        if (pos.column - 1 < 0) {
            return new Matrix.Position(pos.row - 1, columns - 1);
        } else {
            return new Matrix.Position(pos.row, pos.column - 1);
        }
    }

    private Position positionAfter(int rows, int columns, Position pos) {
        if (pos.column + 1 >= columns) {
            return new Matrix.Position(pos.row + 1, 0);
        } else {
            return new Matrix.Position(pos.row, pos.column + 1);
        }
    }
}

Test_Matrix_Position.java

public class Test_Matrix_Position {
    @Test
    public void testCreation() {
        Matrix.Position pos = new Matrix.Position(1, 2);
        assertEquals(1, pos.row);
        assertEquals(2, pos.column);
    }

    @Test(expected=IllegalArgumentException.class)
    public void testIllegalArguments() {
        new Matrix.Position(1, -2);
    }

    @Test
    public void testEquals() {
        assertTrue(new Matrix.Position(1, 2).equals(new Matrix.Position(1, 2)));
        assertFalse(new Matrix.Position(1, 2).equals(new Matrix.Position(0, 0)));
        assertFalse(new Matrix.Position(1, 2).equals(null));
    }
}
\$\endgroup\$
2
\$\begingroup\$

Excellent coding. I don't claim to understand all your code, but your writing style made it easier for me to review.

I have three suggestions. I've found these suggestions to be helpful to me with large Java code bases.

1) Define your class variables in alphabetical order by class. This helps you find a particular variable quickly in a long list of class variables.

Here's what I mean from your PuzzlesView class.

private Bitmap fullImage;

private Canvas canvas;

private Collection<OnGameFinishedListener> onGameFinishedListeners;

private Dimension dim;

private GameArbitrator arbitrator;

private Matrix<Bitmap> puzzles;

private Matrix.Position draggedPosition;
private Matrix.Position nearestPosition;

private Mixer mixer;

private Point lastTouchedPoint;
private Point draggedLeftUpper;

private Size puzzleSize;

2) Related to the first point, initialize class fields in the constructor, not the field definition. Initialize static fields in the field definition.

public PuzzlesView(Context context) {
    super(context);
    this.onGameFinishedListeners =
            new ArrayList<OnGameFinishedListener>();
    this.mixer = new Mixer();
}

3) Java already has a Dimension class. Call your class something like PuzzleDimension. You have a similar problem with the Size class. Every Collections class has a size method. Call your class something like PuzzleSize.

| improve this answer | |
\$\endgroup\$
  • 1
    \$\begingroup\$ Very few remarks indeed. gameFinished() is too deeply nested, as is Matrix' equals(). Other than that readability may be helped by using named inner classes instead of anonymous ones, but that is already nitpicking. \$\endgroup\$ – bowmore Jun 12 '13 at 16:48
  • \$\begingroup\$ To: Gilbert Le Blanc, bowmore. Thank you very much! \$\endgroup\$ – leonideveloper Jun 16 '13 at 6:05

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.