Is this a tidy way of coding? Or is there a prefered method?

[Andy_Wilson]

Hi guys, so I wanted to just add another thing to the number wizard for the hell of it, a simple warning if there was one guess left, not really in the context of the game because it assumes the user is doing the guessing. That’s beside the point, I wondered if the actual coding is a preferred method or not, or is there a better way of doing it? (It works perfectly ok).

I added another function with an if statement, if maxGuessesAllowed == 1 then return message. I then called the function in the next guess function. Am I wasting code, or doing things oddly here? Thanks

void NextGuess (){
	guess = Random.Range (min, max + 1);
	text.text = guess.ToString();
	maxGuessesAllowed = maxGuessesAllowed - 1;
	guessesRemaining = maxGuessesAllowed;
	triesLeft.text = guessesRemaining.ToString();
	WarningMessage ();
	if (maxGuessesAllowed <= 0) {
		SceneManager.LoadScene ("Win");
	}
}

void WarningMessage (){
	if (maxGuessesAllowed == 1) {
		warnings.text = ("One Guess Remaining!!!");
	
	}

}

[Martyn_Stewart]

It looks fine to me. Personally since it’s only 1 if statement and not being used elsewhere I wouldn’t add a new method for it and since there’s only 1 outcome ie it’s true you could just write it on one line as:

	if (maxGuessesAllowed == 1)     warnings.text = ("One Guess Remaining!!!");

[Andy_Wilson]

Ok thanks. I understand it’s really quite a simplified example. I guess its one of those things that will come with experience, at what point its worth writing a new method for a piece of code?

Really, I was just doing it for learning’s sake, this is all new to me, actually understanding how it works! I’ve tried before and failed, but I think that was because I didn’t have the help I needed

[Maxim_Inozemtsev]

It is not that it is a waste of code, it just could be a bit confusing for those who will read your code.
When someone tries to read line by line and encounters this:

WarningMessage();

They would think that you writer warning message all the time, no one would expect you to make checks inside of that method;
In my opinion it would be more readable to rewrite it like this:

void NextGuess (){
    guess = Random.Range (min, max + 1);
    text.text = guess.ToString();
    maxGuessesAllowed = maxGuessesAllowed - 1;
    guessesRemaining = maxGuessesAllowed;
    triesLeft.text = guessesRemaining.ToString();
    if (maxGuessesAllowed == 1) {
       WarningMessage ();
    }
    if (maxGuessesAllowed <= 0) {
        SceneManager.LoadScene ("Win");
    }
}

void WarningMessage ()
{
    warnings.text = ("One Guess Remaining!!!");	
}

Or you could also rename your WarningMessage() to something like WarningMessageIfLastTry() so that again your code would be more readable;

Generally you want to extract some code to standalone function is when you are going to use that code from many places, so you don’t have to rewrite it many times, or just to split function into smaller pieces for readability purposes;

[Andy_Wilson]

Thanks for the advice, that does ring a bell actually, the code reuse statement.