Number Wizard game

Question

This is a number guesser that guesses a random number between the values that you set. The user has to use the up or down arrow keys to say if the number that they thought of between the number generated. This continues until the number that the user is thinking is generated and then the user pressed enter to say that their number has been guessed.

How can I make my code shorter? I feel that like I have a lot of unnecessary code. I mainly want to shorten the Update() area but any and all code shortcuts that could help me would be great.

NOTE: I had to use the newMax and newMin because the max and min values weren't updating correctly when I called them on ↑ and ↓ arrow presses.

using UnityEngine;
using System.Collections;

public class NumberWizard : MonoBehaviour {
    int max;
    int min;
    int guess;
    bool canGuess = false;
    bool setMax = false;
    bool setMin = false;

    // Use this for initialization
    void Start () {
        print ("Welcome to Number Wizard");
        print("Set max number!");
        setMax = true;
    }

    void StartGame () {
        print ("Pick a number in your head, but don't tell me!");

        print ("The highest number you can pick is " + max);
        print ("The lowest number you can pick is " + min);

        max = max +1;
        guess = Random.Range (min, max);

        print ("Is the number higher or lower than " + guess + "?");
    }

    // Update is called once per frame
    void Update () {        
        if (min >= max && !canGuess) min = max - 1;
        if (max > 0 && min < 0 && !canGuess) min = 0;
        if (max < 0) max = 0;

        int newMax = max;
        int newMin = min;

        if (Input.GetKeyDown(KeyCode.UpArrow)){
            if (canGuess){
                min = guess;
                NextGuess ();
            } else if (!canGuess && setMax){
                max += 100;
                newMax += 100;
                print ("Max: " + newMax);
            } else if (!canGuess && setMin){
                min += 1;
                newMin += 1;
                print ("Min: " + newMin);
            }
        } else if (Input.GetKeyDown(KeyCode.DownArrow)){
            if (canGuess){
                max = guess;
                NextGuess ();
            } else if (!canGuess && setMax){
                max -= 100;
                newMax -= 100;
                if (newMax < 0) newMax = 0;
                print ("Max: " + newMax);
            } else if (!canGuess && setMin){
                min -= 1;
                newMin -= 1;
                if (newMin < 0) newMin = 0;
                print ("Min: " + newMin);
            }
        } else if (Input.GetKeyDown(KeyCode.Return)){
            if (canGuess){
                print ("I guessed correct!");
                Start ();
                min = 0;
                max = 0;
                canGuess = false;
            } else if (!canGuess && setMax){
                if (max > 0){
                    print("Set min number!");
                    setMax = false;
                    setMin = true;
                } else if (max == 0)print("Choose a max number!");
            }else if (!canGuess && setMin){
                StartGame ();
                setMin = false;
                canGuess = true;
            }
        }
    }

    void NextGuess () {
        guess = (max + min) / 2;
        print ("Is the number equal to, higher or lower than " + guess + "?");
    }
}

Answer 1

Architecture:

You should really try to abstract out the environment specific aspects (the fact you're using unity) I would suggest creating a stand alone class Game and implementing all the business logic in there. Your class (NumberWizard ) would then become a thin wrapper that is only responsible for instantiation of the Game class and passing commands to it. Doing so would contribute a lot for your solution testability and re-usability.

Business logic and readability:

1. You could try to give a descriptive name to your logical conditions:

   !canGuess && setMax

   would become var isInSetMinState = !canGuess && setMax which would then be used in the if statements. Doing so would make it easier to see what each if statement is for.

2. Your game has 2 major types of actions:
   Increase/Decrease
   Confirm(essentially change state)
   It also has 3 states:
   SetMax
   SetMin
   Guess
   I would try to use them more explicitly in your code. It would make your code more readable and easier to understand.

something like the following:

private enum GameState {SetMax,SetMin,Guess};
private enum Action {None, Increase, Decrease, Confirm};
var _state = GameState.Init
var _action = None;

...

public void Update()
{
    _action = GetAction();
    switch(_action)
    {
        case Action .Confirm:
            switch(_state)
            {
                case GameState.SetMax:
                    _state = GameState.SetMin;
                break;
                case GameState.SetMin:
                   _state = GameState.SetMin;
                break;
                case GameState.Guess:
                   _state = GameState.Win;
               break;
        }
        break;
        case Action.Increase:
            switch(_state)
            {
                case GameState.SetMax:
                    //increase max logic
                break;
                case GameState.SetMin:
                   //increase min logic
                break;
                case GameState.Guess:
                   //guess in an upper range of [guess,max]
                break;
            }
        break;
        case Action.Decrease:
           //same as decrease
    }
}