koobas.hobune.stream
Can I store a value computed in one method outside of that method?
I am trying to access the currentAnswer value from the method shown below to use in another method. Will I have to make it a global variable? I do not want to call the method because it would generate a new random problem and answer. I am in the process of simplifying the method by breaking it down into smaller methods. I am still learning, so any critiques in that department would also be welcomed. Here is the code:
public double GenerateRandomProblem()
{
double firstRandomNumber;
double secondRandomNumber;
double currentAnswer = 0;
string currentOperator = "";
string space = " ";
bool isCurrentAnswerAcceptable = false;
Vector3[] buttonPositions = new Vector3[4];
while (!isCurrentAnswerAcceptable)
{
InitializeButtonPositions();
int currentOperatorCallNumber;
currentOperatorCallNumber = Random.Range(1, 4);
switch (currentOperatorCallNumber)
{
case 1:
currentOperator = "+";
break;
case 2:
currentOperator = "-";
break;
case 3:
currentOperator = "/";
break;
case 4:
currentOperator = "*";
break;
}
firstRandomNumber = Random.Range(1, 10);
secondRandomNumber = Random.Range(1, 10);
switch (currentOperator)
{
case "+":
currentAnswer = firstRandomNumber + secondRandomNumber;
break;
case "-":
currentAnswer = firstRandomNumber - secondRandomNumber;
break;
case "/":
currentAnswer = firstRandomNumber / secondRandomNumber;
break;
case "*":
currentAnswer = firstRandomNumber * secondRandomNumber;
break;
}
bool isCurrentAnswerAboveZero;
if (currentAnswer > 0)
{
isCurrentAnswerAboveZero = true;
}
else
{
isCurrentAnswerAboveZero = false;
}
bool isCurrentAnswerWholeNumber;
if (currentAnswer % 1 == 0)
{
isCurrentAnswerWholeNumber = true;
}
else
{
isCurrentAnswerWholeNumber = false;
}
if (isCurrentAnswerAboveZero == true && isCurrentAnswerWholeNumber == true)
{
isCurrentAnswerAcceptable = true;
}
else
{
isCurrentAnswerAcceptable = false;
}
if (isCurrentAnswerAcceptable)
{
int generateRandomButtonAnswerInt;
double[] intsForButtons = new double[4];
currentProblem.text = firstRandomNumber + space + currentOperator + space + secondRandomNumber;
generateRandomButtonAnswerInt = Random.Range(0, 1);
intsForButtons[0] = currentAnswer;
intsForButtons[1] = currentAnswer - 1;
intsForButtons[2] = currentAnswer + 1;
if (generateRandomButtonAnswerInt == 0)
{
intsForButtons[3] = currentAnswer - 2;
}
if (generateRandomButtonAnswerInt == 1)
{
intsForButtons[3] = currentAnswer + 2;
}
button1.GetComponentInChildren<Text>().text = intsForButtons[0].ToString();
button2.GetComponentInChildren<Text>().text = intsForButtons[1].ToString();
button3.GetComponentInChildren<Text>().text = intsForButtons[2].ToString();
button4.GetComponentInChildren<Text>().text = intsForButtons[3].ToString();
buttonPositions[0] = button1Position;
buttonPositions[1] = button2Position;
buttonPositions[2] = button3Position;
buttonPositions[3] = button4Position;
Shuffle(buttonPositions);
button1.transform.localPosition = buttonPositions[0];
button2.transform.localPosition = buttonPositions[1];
button3.transform.localPosition = buttonPositions[2];
button4.transform.localPosition = buttonPositions[3];
}
}
return currentAnswer;
}
You could have a public variable but you could also return the value with the method return type. Bertrand Meyer devised the Command-query separation principle that could often be helpful to follow especially in your case. It states that a method should either be a command or a query. A command performs an action like calculating a new answer and the query would be to retrieve the current answer. No matter how many times you try to retrieve the answer it remains the same until you send a command to generate a new answer. An easy way of spotting a command is that it does not return anything, so it is always void. A query always has a return type. As for critique on your code I would say that you are trying way to many things in one method for it to be readable. I generally try to keep one method below 50 lines of code and the smaller the better as long as it makes everything more readable. Since you got no comments I have to read all of your code to get any idea of what your intentions are. This will also happen to yourself when you go back and read the code in a month or a year. So comment everything unless if it is really obvious or make it into small methods with good names so you can get the overall idea just by reading the method names. A good method only does one thing, but it does it really well.
Thanks a lot. I recently finished reading code complete, but I hadn't gotten around to implementing what I had learned from it yet. That CQS principle iced the cake with everything I had picked up from that book. I just finished cleaning up that method (creating a bunch of smaller methods), and it is the most elegant code I have written yet. Just thinking about that principle helped me decide on how to structure each method. I still need to go through and comment the code, but I did my best to name everything in a self-explanatory type manner. I'm excited now and completely addicted to program$$anonymous$$g. Thanks again.
I am glad it helped. Na$$anonymous$$g everything can be a challenge in itself. I often have to rename/refactor names several times when coding. It is however enjoyable to create your own "system" that does one specific thing very well. Feel free to post your final result if you want some more feedback.
@Pengocat I wrote in some quick comments, about to board a flight. Some feedback would be great.
using UnityEngine;
using UnityEngine.UI;
using UnityEngine.EventSystems;
using System.Collections;
using System.Collections.Generic;
using System.Linq;
public class GameManagerScript : MonoBehaviour
{
public Button button1;
public Button button2;
public Button button3;
public Button button4;
public Vector3 button1Position;
public Vector3 button2Position;
public Vector3 button3Position;
public Vector3 button4Position;
public int score;
public string space = " ";
List<double> currentTotalList = new List<double>();
public Text currentProblem;
public Text questionResult;
public Text scoreBox;
static System.Random _random = new System.Random();
void Start()
{
DisplayProblem();
}
double GenerateFirstRandomNumber()
{
double firstRandomNumber;
firstRandomNumber = Random.Range(1, 10);
return firstRandomNumber;
}
double GenerateSecondRandomNumber()
{
double secondRandomNumber;
secondRandomNumber = Random.Range(1, 10);
return secondRandomNumber;
}
//this creates a random problem using the two generate random number methods above
string GenerateFirstRandomProblem()
{
double currentAnswer = 0;
double firstRandomNumber;
double secondRandomNumber;
string completeEquation;
firstRandomNumber = GenerateFirstRandomNumber();
secondRandomNumber = GenerateSecondRandomNumber();
string currentOperator = "";
int currentOperatorCallNumber;
currentOperatorCallNumber = Random.Range(1, 5);
//this generates a random operator to be used for the current problem, these strings are also used to display the operator in the question box on game screen
switch (currentOperatorCallNumber)
{
case 1:
currentOperator = "+";
break;
case 2:
currentOperator = "-";
break;
case 3:
currentOperator = "/";
break;
case 4:
currentOperator = "x";
break;
}
//this takes whichever random operator was generated and puts it into use with the two random numbers
switch (currentOperator)
{
case "+":
currentAnswer = firstRandomNumber + secondRandomNumber;
break;
case "-":
currentAnswer = firstRandomNumber - secondRandomNumber;
break;
case "/":
currentAnswer = firstRandomNumber / secondRandomNumber;
break;
case "x":
currentAnswer = firstRandomNumber * secondRandomNumber;
break;
}
//this was converted to a string so that it can be easily read and manipulated later on
completeEquation = firstRandomNumber.ToString() + space + currentOperator.ToString() + space + secondRandomNumber.ToString() + space + currentAnswer.ToString();
return completeEquation;
}
//this method takes in that first random problem that was generated, divides up each component of the equation,
//and then sends the answer to the equation into check current answer method,
//if is accepted under the guidelines laid out in the method, it is returned
double GenerateFinalProblem()
{
bool isAnswerAcceptable = false;
double firstRandomNumber = 0;
double secondRandomNumber = 0;
double currentAnswer = 0;
while (!isAnswerAcceptable)
{
string currentEquation = GenerateFirstRandomProblem();
string firstNumber = currentEquation.Split(' ').First();
string currentOperator = currentEquation.Split(' ')[1];
string secondNumber = currentEquation.Split(' ')[2];
string answer = currentEquation.Split(' ').Last();
firstRandomNumber = double.Parse(firstNumber);
secondRandomNumber = double.Parse(secondNumber);
currentAnswer = double.Parse(answer);
if (CheckCurrentAnswer(currentAnswer))
{
currentProblem.text = firstNumber + space + currentOperator + space + secondNumber;
isAnswerAcceptable = true;
break;
}
}
StorePreviousAnswers(currentAnswer);
return currentAnswer;
}
//this checks to make sure the current answer is a positive whole number
public bool CheckCurrentAnswer(double currentAnswer)
{
bool isCurrentAnswerAcceptable = false;
bool isCurrentAnswerAboveZero;
if (currentAnswer > 0)
{
isCurrentAnswerAboveZero = true;
}
else
{
isCurrentAnswerAboveZero = false;
}
bool isCurrentAnswerWholeNumber;
if (currentAnswer % 1 == 0)
{
isCurrentAnswerWholeNumber = true;
}
else
{
isCurrentAnswerWholeNumber = false;
}
if (isCurrentAnswerAboveZero == true && isCurrentAnswerWholeNumber == true)
{
isCurrentAnswerAcceptable = true;
}
else
{
isCurrentAnswerAcceptable = false;
}
return isCurrentAnswerAcceptable;
}
//If all parameters are met, this will display the current problem
//It will also generate the answer choices to fill the button,
//and then shuffle the buttons around
double DisplayProblem()
{
InitializeButtonPositions();
double currentAnswer = GenerateFinalProblem();
Vector3[] buttonPositions = new Vector3[4];
int generateRandomButtonAnswerInt;
double[] intsForButtons = new double[4];
generateRandomButtonAnswerInt = Random.Range(0, 1);
intsForButtons[0] = currentAnswer;
intsForButtons[1] = currentAnswer - 1;
intsForButtons[2] = currentAnswer + 1;
if (generateRandomButtonAnswerInt == 0)
{
intsForButtons[3] = currentAnswer - 2;
}
if (generateRandomButtonAnswerInt == 1)
{
intsForButtons[3] = currentAnswer + 2;
}
button1.GetComponentInChildren<Text>().text = intsForButtons[0].ToString();
button2.GetComponentInChildren<Text>().text = intsForButtons[1].ToString();
button3.GetComponentInChildren<Text>().text = intsForButtons[2].ToString();
button4.GetComponentInChildren<Text>().text = intsForButtons[3].ToString();
buttonPositions[0] = button1Position;
buttonPositions[1] = button2Position;
buttonPositions[2] = button3Position;
buttonPositions[3] = button4Position;
Shuffle(buttonPositions);
button1.transform.localPosition = buttonPositions[0];
button2.transform.localPosition = buttonPositions[1];
button3.transform.localPosition = buttonPositions[2];
button4.transform.localPosition = buttonPositions[3];
return currentAnswer;
}
//this generates a list of each correct answer, and sums them up
double StorePreviousAnswers(double currentAnswer)
{
currentTotalList.Add(currentAnswer);
double currentTotal;
currentTotal = currentTotalList.Sum();
return currentTotal;
}
//This generates the next problem if the correct answer is selected
void GenerateNextProblem()
{
DisplayProblem();
}
//this plays out when the correct answer is selected and tallies a simple score
public void DisplayCorrectResult()
{
questionResult.text = "Correct";
score++;
scoreBox.text = score.ToString();
GenerateNextProblem();
}
//this waits one second, then closes the application if the wrong answer is selected
public void DisplayIncorrectResult()
{
questionResult.text = "You Lose";
Invoke("QuitApplication", 1);
}
//logic for closing application in editor
void QuitApplication()
{
UnityEditor.EditorApplication.isPlaying = false;
//Application.Quit();
}
//initializes the original positions of each button
void InitializeButtonPositions()
{
button1Position = button1.transform.localPosition;
button2Position = button2.transform.localPosition;
button3Position = button3.transform.localPosition;
button4Position = button4.transform.localPosition;
}
//this is a Fisher-Yates shuffle method
static void Shuffle<T>(T[] array)
{
int n = array.Length;
for (int i = 0; i < n; i++)
{
int r = i + (int)(_random.NextDouble() * (n - i));
T t = array[r];
array[r] = array[i];
array[i] = t;
}
}
}
First of all when you have a script with variables like button1, button2, button3 etc. you should ins$$anonymous$$d make it into a List or an Array. When lines of code are nearly identical it is more simple to make a for or foreach loop that runs through the list. Secondly you are using double when you could have used simply an int or uint. A double use 64-bit of memory and a float uses 32-bit of memory. Wasting memory is not a good practice. An unsigned int uint allows you to set a number between 0 and 4,294,967,295 which should be plenty for you. It however can't be negative. A regular int is between -2,147,483,648 and 2,147,483,648. Thirdly when making a new method it should do more than calling one existing method otherwise you are simply adding extra lines of codes that needs to be managed for no particular reason. I know you are still building the code and learning (like we all are) but just a re$$anonymous$$der that this type of code is not adding to the readability.
double GenerateFirstRandomNumber()
{
double firstRandomNumber;
firstRandomNumber = Random.Range(1, 10);
return firstRandomNumber;
}
When it is the same as writing
Random.Range(1,10)
