Bug in Ben's code shown in Lecture 36

I noticed this because some students are reporting a “code error” that is preventing them from progressing with the course once they hit it (The latest report HERE).

And unfortunately Ben programs a boo boo in: Lecture 36.

The within the code we have two loops that go through each letter of each word, Hidden Word and Guess: using the following loops:

 // loop through all letters in the hidden word
 for (int32 HWChar = 0; HWChar < MyHiddenWordLength; HWChar++) {
     // compare letters against the guess
     for (int32 GChar = 0; GChar < MyHiddenWordLength; GChar++) {
         //if they math then
         if (Guess[GChar] == MyHiddenWord[HWChar]) {

The problem is: If MyHiddenWordLength is 5, and Guess word is a 3 letter word. The GChar loop will try to loop through 5 times anyway because we are using the < MyHiddenWordLength variable within the for loop.

 for (int32 GChar = 0; GChar < MyHiddenWordLength; GChar++) {

What we should be doing, for the Guess, is only looping for each letter within the word; by using Guess.length()

SO where Ben wrote

int32 MyHiddenWordLength = MyHiddenWord.length(); // assuming same length as guess
 
 // loop through all letters in the hidden word
 for (int32 HWChar = 0; HWChar < MyHiddenWordLength; HWChar++) {
     // compare letters against the guess
     for (int32 GChar = 0; GChar < MyHiddenWordLength; GChar++) {
         //if they math then
         if (Guess[GChar] == MyHiddenWord[HWChar]) {
             if (HWChar == GChar) { // they're in the same place
                 BullCowCount.Bulls++; // incriment Bulls
             }
             else {
                 BullCowCount.Cows++; // must be a cow
             }
         }
     }
 }

What he should have written is

int32 MyHiddenWordLength = MyHiddenWord.length(); // assuming same length as guess
int32 GuessLength = Guess.length(); // assuming same length as guess
 
 // loop through all letters in the hidden word
 for (int32 HWChar = 0; HWChar < MyHiddenWordLength; HWChar++) {
     // compare letters against the guess
     for (int32 GChar = 0; GChar < GuessLength; GChar++) {
         //if they math then
         if (Guess[GChar] == MyHiddenWord[HWChar]) {
             if (HWChar == GChar) { // they're in the same place
                 BullCowCount.Bulls++; // incriment Bulls
             }
             else {
                 BullCowCount.Cows++; // must be a cow
             }
         }
     }
 }

 

The difference being these two lines:

int32 GuessLength = Guess.length(); // assuming same length as guess

for (int32 GChar = 0; GChar < GuessLength; GChar++) {

Of course this is not a problem once the ChechGuessValidity() function is written. But its still one of very few places Ben has written poor code and given the complexity of the subject and learning curve he is taking people on, its absolutely forgivable.

I bypassed that problem by adding an extra if statement before the loop that checks if the Guess.length()==MyHiddenWord.length()

As an else I added a cout that informs the user to use a word of the same word length.

Will eventually have to replace it though once we go to the validity check.

//loop through all the letters
int32 HiddenWordLength = MyHiddenWord.length();
int32 GuessWordLength = Guess.length();

if (GuessWordLength == HiddenWordLength) {
	for (int32 i = 0; i < HiddenWordLength; i++) {
		//compare letters against the hidden word
		for (int32 j = 0; j < HiddenWordLength; j++) {
			//if the match
			if (Guess[i] == MyHiddenWord[j]) {
				//check for exact hit
				if (i == j) {
					//increment bulls
					BullCowCount.Bulls++;
				} else {
					//increment cows
					BullCowCount.Cows++;
				}	
			}		
		}
	}
} else {
	std::cout << "Invalid Guess! Please keep your wordlength at " << HiddenWordLength << std::endl;
}

Privacy & Terms