1

So I'm writing a basic MasterMind game that is... mostly functional. However, its exhibiting odd behavior and I'm unsure why.

The idea is that what defines a Code and its behavior is one file, the gameplay is another, and the Main just creates a new game and starts playing. When I initialize the game, the computer creates a new random string of 4 (the "secret code"), as expected; but then once I get input for the User guess, it seems to rewrite the secret code into whatever I've input. Further, my methods for evaluating matches don't work at all, but considering that the secret code keeps changing means that it's not being set to begin with, and I'm unsure why.

All three classes below. Why is my class variable in Game not setting properly and accessible to the other methods?

Main.java

class Main { 
  public static void main(String[] args) {
    Game newGame = new Game();
    newGame.play(); 
  }
}

Code.java

import java.util.Random;
import java.util.HashMap;
import java.util.Collection;
import java.util.ArrayList;
import java.util.Set;

import java.lang.Math;
import java.lang.StringBuilder;

class Code {
  private static HashMap<String,String> PEGS;
  private static ArrayList<String> pegStrings;
  protected static String secretCodeString;


  public static void main(String[] args) {

  }

  public Code(String input){
    this.secretCodeString = input;
  }

  public Code(){
    randomize();
  }


  //literally just creates the peghash
  public static void setPegs(){
    PEGS = new HashMap<String,String>();

    PEGS.put("C","c");
    PEGS.put("Y","y");
    PEGS.put("R","r");
    PEGS.put("P","p");
    PEGS.put("O","o");
    PEGS.put("G","g");
  }

  //turns the pegs ito something randomize can use
 public static ArrayList<String> makePegArray(){
    setPegs();

    pegStrings = new ArrayList<String>();

    Collection<String> pegValues = PEGS.values();
    Object[] pegObjects = pegValues.toArray();

      for (int i = 0; i < pegObjects.length; i++){
        pegStrings.add(pegObjects[i].toString());
      }

    return pegStrings;
  }

  // sets Class Variable secretCode to a four letter combination
  public static Code randomize(){
    secretCodeString = new String();

    Random rand = new Random();
    int randIndex = rand.nextInt(makePegArray().size());

    for (int i = 0; i < 4; i++){
      randIndex = rand.nextInt(makePegArray().size());
      secretCodeString = secretCodeString.concat(makePegArray().get(randIndex));
    }

      Code secretCode = parse(secretCodeString);
      return secretCode;
  }

  public static Code parse(String input) {
    setPegs();
    makePegArray();

    String[] letters = input.split("");
    StringBuilder sb = new StringBuilder();

    for (String letter : letters) {
      if (pegStrings.contains(letter)) {
        sb.append(letter);
      } else {
        System.out.println(letter);
        throw new RuntimeException();

      }
    }

    String pegListString = sb.toString();
    Code parsedCode = new Code(pegListString);
    //System.out.println(parsedCode);
    return parsedCode;

  }

  public int countExactMatches(Code guess){
    String guessString = guess.secretCodeString;

    int exactMatches = 0;

    String[] guessArray = guessString.split("");

    String[] winningCodeArray = (this.secretCodeString).split("");

    for(int i = 0; i < 4; i++){

      if(guessArray[i] == winningCodeArray[i]){
        exactMatches++;
      }
    }
    return exactMatches;
  }

  public int countNearMatches(Code guess) {

    String guessString= guess.secretCodeString;

    HashMap<String,Integer> guessCount = new HashMap<String,Integer>();
    HashMap<String,Integer> secretCodeCount = new HashMap<String,Integer>();

    Set<String> codeKeys = guessCount.keySet();

    int matches = 0;
    int keys = guessCount.keySet().size();


    String[] keyArray = new String[keys];



    for(int i = 0; i < guessString.length(); i++) {
      //removes character from string
      String codeCharacter = String.valueOf(guessString.charAt(i));
      String guessShort = guessString.replace(codeCharacter,"");

      //counts instances of said character
      int count = guessString.length() - guessShort.length();

      guessCount.put(codeCharacter, count);
    }

    for(int i = 0; i < secretCodeString.length(); i++) {
      //removes character from string
      String winningString = this.secretCodeString;

      String winningCodeCharacter = String.valueOf(winningString.charAt(i));
      String winningCodeShort = guessString.replace(winningCodeCharacter,"");

      //counts instances of said character
      int count = winningString.length() - winningCodeShort.length();

      secretCodeCount.put(winningCodeCharacter, count);
    }

    for (int i = 0; i < keys; i++) {
      codeKeys.toArray(keyArray);
      String keyString = keyArray[i];

      if (secretCodeCount.containsKey(keyString)) {
        matches += Math.min(secretCodeCount.get(keyString), guessCount.get(keyString));
      } 
    }

    int nearMatches = matches - countExactMatches(guess);

    return nearMatches;
  }
}

Game.java

import java.util.Scanner;

class Game {

  protected static Code winningCode;

  public static void main(String[] args){

  }

  public Game(){
    winningCode = new Code();
  }

  protected static Code getGuess() {

    Scanner userInput = new Scanner(System.in);

    int count = 0;
    int maxTries = 5;
    while(true){
      try {
        String codeToParse = userInput.next();
        Code guess = Code.parse(codeToParse);
        return guess;

      } catch(RuntimeException notACode) {
        System.out.println("That's not a valid peg. You have " + (maxTries - count) + " tries left.");
        if (++count == maxTries) throw notACode;
      }
    }


  }

  protected static void displayMatches(Code guess){

    int nearMatches = winningCode.countNearMatches(guess);
    int exactMatches = winningCode.countExactMatches(guess);

    System.out.println("You have " + exactMatches + " exact matches and " + nearMatches + " near matches.");
  }

  protected static void play(){
    int turnCount = 0;
    int maxTurns = 10;

    System.out.println("Greetings. Pick your code of four from Y,O,G,P,C,R.");

    while(true){
      Code guess = getGuess();
      displayMatches(guess);

      if (guess == winningCode) {
        System.out.print("You win!!");
        break;
      } else if (++turnCount == maxTurns) {
        System.out.print("You lose!!");
        break;
      }
    }
  }
}
weston
  • 54,145
  • 21
  • 145
  • 203
Andrea McKenzie
  • 239
  • 1
  • 12
  • 4
    Why are you declaring everything as static?? Bad. http://stackoverflow.com/questions/2671496/java-when-to-use-static-methods – OldProgrammer Dec 30 '16 at 00:41
  • I'm very much a new programmer so I'm probably doing this incorrectly. But many of my methods pop up with an error saying that you can't call static from a non-static context or vice versa. I change them as I need to to stop seeing the error – Andrea McKenzie Dec 30 '16 at 00:43
  • 2
    Nope, Nope, Nope. You are doing that because Main() is declared static. You need to create an instance of the class (object) in Main(), then reference methods through the object. Read link in comment. – OldProgrammer Dec 30 '16 at 00:49
  • ...That makes infinitely more sense than what I was doing. So would that explain why winningCode keeps "rewriting" itself? Because it's not singularly establishing the object? – Andrea McKenzie Dec 30 '16 at 00:52
  • You shouldn't use RuntimeException to handle incorrect user input. https://stackoverflow.com/questions/3213094/when-to-use-exceptions-in-java-example Also see https://stackoverflow.com/questions/2279030/type-list-vs-type-arraylist-in-java – MWin123 Dec 30 '16 at 01:01

2 Answers2

3

On every guess, you call Code.parse, Code.parse creates a new Code (new Code(pegListString);) and that constructor sets the secretCodeString and because that's static, all instances of Code share the same variable. You need to avoid mutable static members.

Another tip is to either have a method return a value, or mutate state (of either its input, or its own instance, this), but avoid doing both.

weston
  • 54,145
  • 21
  • 145
  • 203
  • I see what you mean, but I have a second constructor there that takes input and is supposed to set the code's String value to the argument provided `this.secretCodeString = input`. Is that invalid now that there's the randomize/no param constructor as well? – Andrea McKenzie Dec 30 '16 at 01:28
  • I see, I missed that, well it's still similar to what I thought still, as both constructors change the shared static field `secretCodeString`. – weston Dec 30 '16 at 03:56
1

"Why is my class variable rewriting itself after an unrelated method runs?"

Because, actually, it is not unrelated. The "mess" that you have created by declaring variables and methods as static has lead to unwanted coupling between different parts of your code.

It is difficult to say what the correct solution is here because your code has gotten so confused by the rewrites that it is hard to discern the original "design intent".

My advice would be to start again. You now should have a clearer idea of what functionality is required. What you need to do is to redo the object design so that each class has a clear purpose. (The Main and Game classes make sense, but Code seems to be a mashup of functionality and state that has no coherent purpose.)

Stephen C
  • 698,415
  • 94
  • 811
  • 1,216