Problem
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 + "?");
}
}
Solution
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:
-
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 theif
statements. Doing so would make it easier to see what eachif
statement is for. -
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
}
}