Java 3x3 Scrambler

[Elliot]

This is a scramble generating program that I wrote in Java. This is my first time creating a scrambler, so any suggestions about correctness, style, efficiency, etc. would be greatly appreciated. Thanks!

/**
 * Scrambler.java 
 * 
 * Rubik's Cube scramble generator
 * 
 * @author Elliot Penson
 */

public class Scrambler {

    /**
     * The faces of a Rubik's Cube represented as side/axis
     */
    private static final String[] faces = { "RX", "LX", "UY", "DY", "FZ", "BZ" };
	
    /**
     * Main method
     * @param args
     */
    public static void main(String[] args) {
        for(int i = 0; i < Integer.parseInt(args[0]); i++)
            System.out.println(generateScramble());
    }
	
    /**
     * Generates a random 25 move scramble
     * @return 
     */
    public static String generateScramble() {
        String scramble = "";
        String lastFace = "  ";

        for(int i = 0; i < 25; i++)
            scramble += (lastFace = randomFace(lastFace)).charAt(0) + randomDirection() + " ";

        return scramble;
    }

    /**
     * Finds a random face that is not on the same axis as the previous turn.
     * @param lastFace
     * @return
     */
    public static String randomFace(String lastFace) {
        String toReturn = faces[(int)(Math.random() * faces.length)];
        if(toReturn.charAt(1) == lastFace.charAt(1))
            return randomFace(lastFace);
        return toReturn;
    }
	
    /**
     * Returns a random direction for a face rotation
     * @return
     */
    public static String randomDirection() {
        switch((int)(Math.random() * 3)) {
            case 0: return "";  
            case 1: return "'";
            default: return "2";
        }
    }
}

 

[PatrickJameson]

Finds a random face that is not on the same axis as the previous turn.

RL is a valid scramble, and this scrambler doesn't seem to allow that. I'll leave you to figure that one out .

The structure of the code is fine though, and interesting way of keeping track of the axis.

Also, instead of using a switch statement, you can use an array and have the index be a random integer from 0-2, just like you did with faces.

 

[mariano.aquino]

nice coding! one more thing to take into account, RL is valid, however RLR is not. So it is not that easy as it seems =)
I'll share mine when I finish it , too!

 

[Veerexx]

I like ^_^ :3

 

[Elliot]

Thanks for the help guys. The scramble generator now does not allow moves that were the same as the last, or on the same axis as the last three. I hope this then allows the valid R L set of moves. Here is my updated code. Let me know of any other changes you would recommend.

/**
 * Scrambler.java 
 * 
 * Rubik's Cube scramble generator
 * 
 * @author Elliot Penson
 */

public class Scrambler {

    /**
     * The faces of a Rubik's Cube represented as side/axis
     */
    private static final String[] faces = { "RX", "LX", "UY", "DY", "FZ", "BZ" };

    /**
     * Possible directions of a face turn
     */
    private static final String[] rotation = { "", "'", "2" };
    
    /**
     * Main method for testing purposes
     * @param args
     */
    public static void main(String[] args) {
        for(int i = 0; i < Integer.parseInt(args[0]); i++) {
            System.out.println(generateScramble());
        }
    }

    /**
     * Generates a random 25 move scramble
     * @return 
     */
    public static String generateScramble() {
        String scramble = "";
        
        String penultimateFace = "  ";
        String lastFace = "  ";

        for(int i = 0; i < 25; i++) {
            String newFace = randomFace(penultimateFace, lastFace);
            scramble += newFace.charAt(0) + randomDirection() + " ";
            penultimateFace = lastFace;
            lastFace = newFace;
        }

        return scramble;
    }

    /**
     * Finds a random face that is not on the same axis as the previous turn.
     * @param lastFace
     * @return
     */
    public static String randomFace(String penultimate, String last) {
        String toReturn = faces[(int)(Math.random() * faces.length)];
        if(last.equals(toReturn) || sameAxis(penultimate, last, toReturn))
            return randomFace(penultimate, last);
        return toReturn;
    }
    
    /**
     * Compares three face's axes
     * @return true if all the Strings have the same axis
     */
    public static boolean sameAxis(String a, String b, String c) {
        return a.charAt(1) == b.charAt(1) && b.charAt(1) == c.charAt(1);
    }

    /**
     * Returns a random direction for a face rotation
     * @return
     */
    public static String randomDirection() {
        return rotation[(int)(Math.random() * 3)];
    }

}

 

[tim]

    private static final String[] faces = { "RX", "LX", "UY", "DY", "FZ", "BZ" };

    // [...]

        return a.charAt(1) == b.charAt(1) && b.charAt(1) == c.charAt(1);
}

That's indeed pretty clever, but in my opinion too clever. The first time reading your code I started somewhere in the middle and ended up being super confused by faces[0].charAt(1) being a thing. I'd either
a) extract the notion of a face into a separate class having fields for axis(int/string/enum) and face(string/char) — with the benefit of replacing "oldFace.charAt(1)" with "oldFace.axis" — or
b) nest faces within axes, e.g. axes[0].faces[1].letter // => "L"

edit: Another indicator that that's actually a code smell:

String lastFace = "";

this (intuitive) definition would just break your code.

Introducing objects has one big drawback, though:

Face lastFace = null;

You'd either have to check for null every time you use lastFace or implement a null object/create a non-existing face.

 

[Elliot]

Thanks for the tips tim! I made some updates to follow your suggestions. Check them out here: https://github.com/ElliotPenson/CubeScrambler

 

[MarcelP]

Your switch((int)(Math.random() * 3)) is generated without a 'seed'. That means, if you start the program e few times after each other in a short period you will get the same result. Instead in the constructor you should do a seed.

So make a private Random random = new Random(Integer.MAX_INT); //private in your Scrambler class

End the simplest way to get your 0,1 or 2 value is

random.getNextInt(3);

 

[tim]

Elliot: Nice! It's pretty readable now.

So make a private Random random = new Random(Integer.MAX_VALUE); //private in your Scrambler class

This will set the seed to the same value every time you start your application, therefore producing the exact same "random" outputs every time — see this stackoverflow answer. If you want to fix this problem just set the seed to the current system time (relevant stackoverflow question).

/edit: After digging deeper into the docs: Math.random() initially creates a new Random object by calling new Random() which internally sets the seed to the current time in milliseconds in 1.4 or "sets the seed of the random number generator to a value very likely to be distinct from any other invocation of this constructor" in Java 6. So, calling Math.random() is fine most of the time (especially for scramblers).

 

[MarcelP]

Oh yep, you are right, better would be something like

private Random random = new Random(Calender.getInstance().getTimeInMiliseconds());

 

[Elliot]

Made some updates to reflect your suggestions. Decided to go with Random rather than Math.random() as per this link. The seed for the random generator is now System.currentTimeMilllis().

 

[Toddyt1]

Oh yep, you are right, better would be something like

private Random random = new Random(Calender.getInstance().getTimeInMiliseconds());

I don't believe this is necessary. Random random = new Random(); Should suffice.

In relation the scrambler. I also just embarked on making one in java (as well as a timer). It works ok (doesn't generate a turn of the same face twice in a row). Are there any other things a generated scramble should/should not have?

 

[mDiPalma]

Are there any other things a generated scramble should/should not have?

prevent RLR

 

[Toddyt1]

prevent RLR

I think I've managed that.