patternjavaModerate
Simple Mastermind game
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
```
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) ||
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.