3
\$\begingroup\$

I have this Java implementation of a Rubik's cube's state. My primary concern is DRYness of my code:

RubiksCubeNode.java

package net.coderodde.libid;

import java.util.Arrays;
import java.util.HashSet;
import java.util.Set;

/**
 *
 * @author Rodion "rodde" Efremov
 * @version 1.6 (May 4, 2019)
 */
public final class RubiksCubeNode {

    public enum Axis {
        X,
        Y,
        Z
    }

    public enum Direction {
        CLOCKWISE,
        COUNTER_CLOCKWISE
    }

    public enum Layer {
        LAYER_1(0),
        LAYER_2(1),
        LAYER_3(2);

        private final int layerNumber;

        private Layer(int layerNumber) {
            this.layerNumber = layerNumber;
        }

        public int toInteger() {
            return layerNumber;
        }
    }

    private final int[][][] data;

    private Set<RubiksCubeNode> neighbors;

    public RubiksCubeNode() {
        data = new int[3][3][3];

        for (int x = 0, value = 0; x < 3; x++) {
            for (int y = 0; y < 3; y++) {
                for (int z = 0; z < 3; z++) {
                    data[x][y][z] = value++;
                }
            }
        }
    }

    public RubiksCubeNode(RubiksCubeNode other) {
        this.data = new int[3][3][3];

        for (int x = 0; x < 3; x++) {
            for (int y = 0; y < 3; y++) {
                for (int z = 0; z < 3; z++) {
                    data[x][y][z] = other.data[x][y][z];
                }
            }
        }
    }

    public RubiksCubeNode rotate(Axis axis,
                                 Layer layer,
                                 Direction direction) {
        RubiksCubeNode nextNode = new RubiksCubeNode(this);

        switch (axis) {
            case X:
                rotateAroundXAxis(nextNode, direction, layer.toInteger());
                break;

            case Y:
                rotateAroundYAxis(nextNode, direction, layer.toInteger());
                break;

            case Z:
                rotateAroundZAxis(nextNode, direction, layer.toInteger());
                break;

            default:
                throw new IllegalArgumentException("Unknown axis enum.");
        }

        return nextNode;
    }

    public Set<RubiksCubeNode> computeNeighbors() {
        if (neighbors != null) {
            return neighbors;
        }

        neighbors = new HashSet<>(18);

        for (Layer layer : Layer.values()) {
            for (Axis axis : Axis.values()) {
                for (Direction direction : Direction.values()) {
                    neighbors.add(rotate(axis, layer, direction));
                }
            }
        }

        return neighbors;
    }

    @Override
    public boolean equals(Object o) {
        if (o == null) {
            return false;
        }

        if (o == this) {
            return true;
        }

        if (!o.getClass().equals(this.getClass())) {
            return false;
        }

        RubiksCubeNode other = (RubiksCubeNode) o;

        for (int x = 0; x < 3; x++) {
            for (int y = 0; y < 3; y++) {
                for (int z = 0; z < 3; z++) {
                    if (data[x][y][z] != other.data[x][y][z]) {
                        return false;
                    }
                }
            }
        }

        return true;
    }

    @Override
    public int hashCode() {
        int i = 1;
        int hash = 0;

        for (int x = 0; x < 3; x++) {
            for (int y = 0; y < 3; y++) {
                for (int z = 0; z < 3; z++) {
                    hash += data[x][y][z] * i++;
                }
            }
        }

        return hash;
    }

    private static void rotateAroundXAxis(RubiksCubeNode node,
                                          Direction direction,
                                          int xLayer) {
        switch (direction) {
            case CLOCKWISE:
                rotateAroundXAxisClockwise(node, xLayer);
                break;

            case COUNTER_CLOCKWISE:
                rotateAroundXAxisCounterClockwise(node, xLayer);
                break;
        }
    }

    private static void rotateAroundYAxis(RubiksCubeNode node,
                                          Direction direction,
                                          int yLayer) {
        switch (direction) {
            case CLOCKWISE:
                rotateAroundYAxisClockwise(node, yLayer);
                break;

            case COUNTER_CLOCKWISE:
                rotateAroundYAxisCounterClockwise(node, yLayer);
                break;
        }
    }

    private static void rotateAroundZAxis(RubiksCubeNode node,
                                          Direction direction,
                                          int zLayer) {
        switch (direction) {
            case CLOCKWISE:
                rotateAroundZAxisClockwise(node, zLayer);
                break;

            case COUNTER_CLOCKWISE:
                rotateAroundZAxisCounterClockwise(node, zLayer);
                break;
        }
    }


    private static void rotateAroundXAxisClockwise(RubiksCubeNode node,
                                                   int xLayer) {
        int[][][] data = node.data;
        int saveValue1 = data[xLayer][0][0];
        int saveValue2 = data[xLayer][1][0];

        data[xLayer][1][0] = data[xLayer][0][1];
        data[xLayer][0][0] = data[xLayer][0][2];
        data[xLayer][0][1] = data[xLayer][1][2];
        data[xLayer][0][2] = data[xLayer][2][2];

        data[xLayer][1][2] = data[xLayer][2][1];
        data[xLayer][2][2] = data[xLayer][2][0];
        data[xLayer][2][1] = saveValue2;
        data[xLayer][2][0] = saveValue1;
    }

    private static void rotateAroundXAxisCounterClockwise(RubiksCubeNode node,
                                                          int xLayer) {
        int[][][] data = node.data;
        int saveValue1 = data[xLayer][0][0];
        int saveValue2 = data[xLayer][1][0];

        data[xLayer][0][0] = data[xLayer][2][0];
        data[xLayer][1][0] = data[xLayer][2][1];
        data[xLayer][2][0] = data[xLayer][2][2];
        data[xLayer][2][1] = data[xLayer][1][2];

        data[xLayer][2][2] = data[xLayer][0][2];
        data[xLayer][1][2] = data[xLayer][2][1];
        data[xLayer][0][2] = saveValue1;
        data[xLayer][0][1] = saveValue2;
    }

    private static void rotateAroundYAxisClockwise(RubiksCubeNode node, 
                                                   int yLayer) {
        int[][][] data = node.data;
        int saveValue1 = data[0][yLayer][0];
        int saveValue2 = data[0][yLayer][1];

        data[0][yLayer][1] = data[1][yLayer][0];
        data[0][yLayer][0] = data[2][yLayer][0];
        data[1][yLayer][0] = data[2][yLayer][1];
        data[2][yLayer][0] = data[2][yLayer][2];

        data[2][yLayer][1] = data[1][yLayer][2];
        data[2][yLayer][2] = data[0][yLayer][2];
        data[1][yLayer][2] = saveValue2;
        data[0][yLayer][2] = saveValue1;
    }

    private static void rotateAroundYAxisCounterClockwise(RubiksCubeNode node,
                                                          int yLayer) {
        int[][][] data = node.data;
        int saveValue1 = data[0][yLayer][0];
        int saveValue2 = data[0][yLayer][1];

        data[0][yLayer][0] = data[0][yLayer][2];
        data[0][yLayer][1] = data[1][yLayer][2];
        data[0][yLayer][2] = data[2][yLayer][2];
        data[1][yLayer][2] = data[2][yLayer][1];

        data[2][yLayer][2] = data[2][yLayer][0];
        data[2][yLayer][1] = data[1][yLayer][0];
        data[2][yLayer][0] = saveValue1;
        data[1][yLayer][0] = saveValue2;
    }

    private static void rotateAroundZAxisClockwise(RubiksCubeNode node,
                                                   int zLayer) {
        int[][][] data = node.data;
        int saveValue1 = data[0][0][zLayer];
        int saveValue2 = data[0][1][zLayer];

        data[0][1][zLayer] = data[1][0][zLayer];
        data[0][0][zLayer] = data[2][0][zLayer];
        data[1][0][zLayer] = data[2][1][zLayer];
        data[2][0][zLayer] = data[2][2][zLayer];

        data[2][1][zLayer] = data[1][2][zLayer];
        data[2][2][zLayer] = data[0][2][zLayer];
        data[1][2][zLayer] = saveValue2;
        data[0][2][zLayer] = saveValue1;
    }

    private static void rotateAroundZAxisCounterClockwise(RubiksCubeNode node,
                                                          int zLayer) {
        int[][][] data = node.data;
        int saveValue1 = data[0][0][zLayer];
        int saveValue2 = data[0][1][zLayer];

        data[0][0][zLayer] = data[0][2][zLayer];
        data[0][1][zLayer] = data[1][2][zLayer];
        data[0][2][zLayer] = data[2][2][zLayer];
        data[1][2][zLayer] = data[2][1][zLayer];

        data[2][2][zLayer] = data[2][0][zLayer];
        data[2][1][zLayer] = data[1][0][zLayer];
        data[2][0][zLayer] = saveValue1;
        data[1][0][zLayer] = saveValue2;
    }
}

RubiksCubeNodeTest

package net.coderodde.libid;

import junit.framework.TestCase;
import static org.junit.Assert.assertNotEquals;
import org.junit.Test;

/**
 * Tests the {@link RubiksCubeNode}. 
 * 
 * @author Rodion "rodde" Efremov
 * @version 1.6 (May 6, 2019)
 */
public class RubiksCubeNodeTest extends TestCase {

    @Test
    public void testRotation1() {
        RubiksCubeNode node = new RubiksCubeNode();
        node = node.rotate(RubiksCubeNode.Axis.X, 
                           RubiksCubeNode.Layer.LAYER_1,
                           RubiksCubeNode.Direction.CLOCKWISE);
        node = node.rotate(RubiksCubeNode.Axis.Z, 
                           RubiksCubeNode.Layer.LAYER_2, 
                           RubiksCubeNode.Direction.COUNTER_CLOCKWISE);
        RubiksCubeNode another = new RubiksCubeNode(node);
        assertEquals(node, another);
        assertEquals(another, node);
        assertEquals(node.hashCode(), another.hashCode());
        RubiksCubeNode third = new RubiksCubeNode(node);
        third = third.rotate(RubiksCubeNode.Axis.Y, 
                             RubiksCubeNode.Layer.LAYER_3,
                             RubiksCubeNode.Direction.CLOCKWISE);
        assertNotEquals(third, node);
        assertNotEquals(third, another);
        assertNotEquals(node.hashCode(), third.hashCode());
    }
}

(The idea for the class RubiksCubeNode is that the configurations comprise an undirected, unweighted graph. We put an edge between any two configurations if they are one rotation away from each other.)

For demonstration, consider forking this GitHub repository.

Typical output looks like this:


Bidirectional BFS path (57684 ms):
          net.coderodde.libid.RubiksCubeNode@1266
          net.coderodde.libid.RubiksCubeNode@1112
          net.coderodde.libid.RubiksCubeNode@12da
          net.coderodde.libid.RubiksCubeNode@12cc
          net.coderodde.libid.RubiksCubeNode@1488
          net.coderodde.libid.RubiksCubeNode@17ac
          net.coderodde.libid.RubiksCubeNode@1998
BIDDFS path (644 ms):
          net.coderodde.libid.RubiksCubeNode@1266
          net.coderodde.libid.RubiksCubeNode@1112
          net.coderodde.libid.RubiksCubeNode@12da
          net.coderodde.libid.RubiksCubeNode@12cc
          net.coderodde.libid.RubiksCubeNode@1488
          net.coderodde.libid.RubiksCubeNode@17ac
          net.coderodde.libid.RubiksCubeNode@1998
Algorithms returns correct paths: true
\$\endgroup\$
  • \$\begingroup\$ i hope i don't step into the same pitfall as last time - but why do you use int[][][] instead of proper Data[][][]? is it that 'competive style' thing again? even then, you could name it better. maybe int[][][] colorSet instead of int[][][] data? - i know it doesn't concern DRY, so it's just a comment ^_^ - happy coding!! \$\endgroup\$ – Martin Frank May 9 at 8:22
1
\$\begingroup\$

I have two comments:

  1. regarding DRYness: this is how I would do it: I see that rotation involves assigning values to 8 cells in the 3d array. I would have rotate() calculate the source and target indices of the rotation based on args. then the actual assignment can be performed in one place. now, when you put all index-calculations in one place, you start noticing recurring patterns in all the various rotations so you can further reduce code duplication

  2. there are two problems with the overall design: first, I don't like the static rotation methods. I mean, it seems right to me that a cube class should be able to rotate itself. this relates to the 2nd point - IIUC, the RubiksCubeNode is intended to be node in graph. but it is also a RubiksCube that does all the cube's operations. this violates the single responsibility principle. a GraphNode is the buliding block of the graph and I would expect it to perform graph related operations, like traversal etc. the data of the GraphNode is an instance of RubiKCube that holds a snapshot of the cube and can perform cube related operations

\$\endgroup\$

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.