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

Simple Mastermind game

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

Problem

This is pretty much my first "bigger" application besides some "Hello World!" stuff, so I would appreciate critiques to improve my style.

It can be started with Mastermind.play(), with being a number from 1 (easy) to 10 (hard).

```
public class Mastermind {
public static void play(int difficulty) {
int round = 1;
int[] zahl = getZufallsZahl(difficulty);
int[] tries = Try(round,difficulty);
int[] hits = getHits(zahl,tries);
while (hits[0]!=difficulty) {
System.out.println("Direct hits: "+hits[0]+"\nIndirect hits: "+hits[1]);
round++;
if (round%(int)(Math.random()*10)==0) {
System.out.println(motivator[(int)(Math.random()*(motivator.length))]);
}
tries = Try(round,difficulty);
hits = getHits(zahl,tries);
}
System.out.println("Congratulations! You got it in "+round+" rounds!");
}
public static int[] getZufallsZahl(int difficulty) {
if (difficulty>10) {
difficulty=10;
}
int[] zahl = new int[difficulty];
int same=1;
for (int i=0; i=0; j--) {
if (zahl[i]==zahl[j]) {
same=1;
break;
}
}
} while (same==1);
}
return zahl;
}

// Below function is copy pasted! :O (yes I am ashamed for it, but it's "only" a niche-feature, so yeah :/)
private static String ordinal(int i) {
String[] sufixes = new String[] { "th", "st", "nd", "rd", "th", "th", "th", "th", "th", "th" };
switch (i % 100) {
case 11:
case 12:
case 13:
return i + "th";
default:
return i + sufixes[i % 10];
}
}
// Above function is copy pasted! :O

private static int[] Try(int round, int difficulty) {
int[] intry = evalTry(getTry(round));
while (!verifyTry(intry) ||

Solution

public class Mastermind {
    public static void play(int difficulty) {
        int round = 1;
        int[] zahl = getZufallsZahl(difficulty);


zahl? What's that?

int[] tries = Try(round,difficulty);


Usually we have classes capitalized, with methods have the first letter lowercase. Because you don't follow convention, that makes this bit harder to read

int[] hits = getHits(zahl,tries);


Ok, but tries implies multiple tries. This is a single attempt, and the multiplicity is in the code assignment.

while (hits[0]!=difficulty) {
            System.out.println("Direct hits: "+hits[0]+"\nIndirect hits: "+hits[1]);
            round++;
            if (round%(int)(Math.random()*10)==0) {
                System.out.println(motivator[(int)(Math.random()*(motivator.length))]);
            }
            tries = Try(round,difficulty);
            hits = getHits(zahl,tries);


You've repeated code before and in the loop. If you use a break in the middle of the loop, you can avoid that. (If somebody told you not to use break, they were wrong).

}
        System.out.println("Congratulations! You got it in "+round+" rounds!");
    }

    public static int[] getZufallsZahl(int difficulty) {
        if (difficulty>10) {
            difficulty=10;
        }
        int[] zahl = new int[difficulty];
        int same=1;
        for (int i=0; i=0; j--) {
                    if (zahl[i]==zahl[j]) {
                        same=1;
                        break;
                    }
                }
            } while (same==1);
        }


Yeah... Don't do that. Instead:

  • Start with an array [0,1,2,3,4,5,6,7,8,9]



  • Shuffle the array



  • Shrink the array to the correct size.



That'll be easier to follow

return zahl;
    }

    // Below function is copy pasted! :O (yes I am ashamed for it, but it's "only" a niche-feature, so yeah :/)


Nothing wrong with copy-pasting. You should take advantage of code that other people wrote as much as possible. The problem with copy-paste is multiple copies of the same code in your program, not taking working code from elsewhere.

private static String ordinal(int i) {
        String[] sufixes = new String[] { "th", "st", "nd", "rd", "th", "th", "th", "th", "th", "th" };
        switch (i % 100) {
        case 11:
        case 12:
        case 13:
            return i + "th";
        default:
            return i + sufixes[i % 10];
        }
    }
    // Above function is copy pasted! :O

    private static int[] Try(int round, int difficulty) {
        int[] intry = evalTry(getTry(round));


intry? What does that mean? Try to pick helpful variable names

while (!verifyTry(intry) || intry.lengthdifficulty) {
            System.out.println("Invalid Input!");
            intry = evalTry(getTry(round));
        }


As before, don't duplicate the contents of the loop before the loop.

// This sort of copying of the array makes sure that a 3 element array becomes a 4 element array with a leading 0 to make it easier to process it afterwards.


What? That's a confusing way to explain what you are doing here. Just say that you are filling the array with leading zeros.

int[] tries = new int[difficulty];
        for (int i=(intry.length-1), k=(difficulty-1); i>=0; k--,i--) {
            tries[k]=intry[i];


What if intry is too long rather then too short?

}
        return tries;
    }
    private static String getTry(int round) {
        BufferedReader br = new BufferedReader(new InputStreamReader(System.in));
        String in = "";
        System.out.print("This is your "+ordinal(round)+" try: ");
        try {
            in = br.readLine();
        } catch (IOException ex) {
            Logger.getLogger(main.class.getName()).log(Level.SEVERE, null, ex);


For quick and dirty application, I suggest throw new RuntimeException(ex); log messages are liable to be missed.

}
        return in;
    }
    private static int[] evalTry(String in) {
        int[] intry = new int[in.length()];
        for (int i=0; i=0; j--) {


Why do you keep making your loops backwards for no reason?

if (intry[i] == intry[j]) {
                    return false;
                }
            }
        }
        return true;
    }
    private static int[] getHits(int[] zahl,int[] tries) {
        // 0 for direct, 1 for indirect.
        int[] hits = new int[2];


I'd suggest storing the hits in two variables rather then as a two element array. Just stick them in the array when you return.

```
for (int i=0; i<zahl.length; i++) {
if (zahl[i]==tries[i]) {
hits[0]++;
}
}
for (int i=0; i<zahl.length; i++) {
for (int k=0; k<zahl.length; k++) {
if (zahl[i]==tries[k] && i!=k) {
hits[1]++;
}
}
}
return hits;
}

private static String[] motivator={"You can do it!","Think... Thin

Code Snippets

public class Mastermind {
    public static void play(int difficulty) {
        int round = 1;
        int[] zahl = getZufallsZahl(difficulty);
int[] tries = Try(round,difficulty);
int[] hits = getHits(zahl,tries);
while (hits[0]!=difficulty) {
            System.out.println("Direct hits: "+hits[0]+"\nIndirect hits: "+hits[1]);
            round++;
            if (round%(int)(Math.random()*10)==0) {
                System.out.println(motivator[(int)(Math.random()*(motivator.length))]);
            }
            tries = Try(round,difficulty);
            hits = getHits(zahl,tries);
}
        System.out.println("Congratulations! You got it in "+round+" rounds!");
    }

    public static int[] getZufallsZahl(int difficulty) {
        if (difficulty>10) {
            difficulty=10;
        }
        int[] zahl = new int[difficulty];
        int same=1;
        for (int i=0; i<difficulty; i++) {
            // This loop is quite ugly because of the check over the same variable, but I didn't want to use a break with a label (outerloop: for [...] break outerloop;)
            do {
                same=0;
                zahl[i]=(int)(Math.random()*10);
                for (int j=i-1; j>=0; j--) {
                    if (zahl[i]==zahl[j]) {
                        same=1;
                        break;
                    }
                }
            } while (same==1);
        }

Context

StackExchange Code Review Q#10475, answer score: 13

Revisions (0)

No revisions yet.