Bulls and Cows out parameters

Unreal Courses Show
,
#1

My Version. A we bit different.

void UBullCowCartridge::GetBullsAndCows(const FString& Guess, int32& BullCount, int32& CowCount)
{
	BullCount = 0;
	CowCount = 0;

	for (int32 i = 0; i < Guess.Len(); i++) {
		if (Guess[i] == HiddenWord[i]) {
			BullCount++; // Direct hit
		}
		else {
			for (int32 j = 0; j < Guess.Len(); j++) 
			{	// See if char is repeated elsewhere.
				if ((Guess[i]) == HiddenWord[j] && (i != j))
				{
					CowCount++;
				}
			}
		}
	}
}
3 Likes
#2

You don’t need to check && (i != j) because You have already checked it in if (Guess[i] == HiddenWord[i]) :blush:

#3

I believe you are right.

Thanks well spotted.

#4

I just wanted to make people aware of a bug in this code.

for (int32 GuessIndex = 0; GuessIndex < Guess.Len(); GuessIndex++){
     if (Guess[GuessIndex] == HiddenWord[GuessIndex]) { // BUG: index out of bounds exception
                ++Count.Bulls;
                continue;
     }
...

The problem is that if Guess is longer than HiddenWord, and we are iterating the for loop through all of the characters in Guess, we could potentially attempt to access more characters than are available in HiddenWord.

For Example:
Say HiddenWord is apple and Guess is apples. Iterating through the for loop for Guess and we get to the ‘s’ in apples which would be Guess[5]. HiddenWord[5] doesn’t exist and as soon as we try to access it as in the above if statement, we get the out of bounds exception.

A simple fix could be to include an additional check to make sure that GuessIndex isn’t larger than HiddenWord.Len() before trying to use it as an index to the character array of HiddenWord.

Or to also include exception handling as I have done in my own code.

3 Likes
#5

This is why we implemented in the “Early Returns” video processing of the input to make sure it is valid.
So in our actual code structure you’ll never get that far down if you guess too many letters.
After checking the code is valid we no longer have to worry about it.

If you plan on calling that function somewhere else in the code which could by pass the validity check, then you can choose to do a check before the for loop, but, since we’ve already handled this it should never be an issue.

#6

Since FString is a collection I decided to make use of version of FOR loop intended for iteration through collection:

void UBullCowCartridge::GetBullsCows (const FString& Guess, int32& BullsGuessed, int32& CowsGuessed)
{
    BullsGuessed=0;
    CowsGuessed=0;
    for (int32 GuessIndex = 0; GuessIndex < Guess.Len(); GuessIndex++)
    {
        if (Guess[GuessIndex]==HiddenWord[GuessIndex])
        {
            BullsGuessed++;
            continue;
        }
        for (auto &&Letter : HiddenWord)
        {
            if (Letter==Guess[GuessIndex]) 
            {
                CowsGuessed++;
            }
        }    
    }   
}

Privacy & Terms