Number Wizard game

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 ("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 + "?");
}
}
• Don't know about shorter, but using flags and putting everything inside the ifs looks very ugly and the logic is hard to determine from it. May 21 '15 at 10:06

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.

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
}
}
• @jozza710 I just posted my version of the same game. You might want to take a look at it. May 24 '15 at 3:55