Thread: C++ compare two arrays and display duplicates

Hello all, I am working on a practice textbook question that asks a user to input 10 characters that go into an array and then that array is compared to a hard-coded alphabet array. The output should display the number of duplicates, if any, per letter. For example:
"There are 2 a's."
"There are 0 b's."
"There are 3 c's." .....and so on.
My code is correctly counting the number of duplicates (or not) between the 2 arrays. However, the problem is that it is displaying the count EVERY TIME THE LOOP ITERATES. I only need it to display THE TOTAL COUNT.
I tried moving the "cout" statement below the loop which doesn't work because it needs the [i] and [j] from where it loops thru the arrays.
Please point out where my error is, thanks in advance!

Code:

#include <iostream> 
#include <iomanip> 
#include <string> 
#include <algorithm> 
#include <Windows.h> 
  
using namespace std;  
  
void parseBuffer( char buffArray[], char alphaArray[], int sizeOne, int sizeTwo ); 
  
int main()  
{ 
 // precode alphabet into an array with null terminating character 
    char alphabetArray[]={'a','b','c','d','e','f','g','h','i','j','k','l','m',n', 
'o','p','q','r','s','t','u','v','w','x','y','z','\0'}; 
  
    char buffer[11]; 
    cout << "Enter the string up to 10 letters." << endl; 
    cin.get(buffer, 11); 
    parseBuffer(buffer, alphabetArray, 11, 11); 
  
    system("Pause"); 
    return 0; 
  
}  
void parseBuffer(char buffArray[], char alphaArray[], int sizeOne, int sizeTwo) 
{ 
  int countFreq = 0; 
  cout << "This is buffer array: " << buffArray << endl; 
  cout << "This is alphabet array: " << alphaArray << endl<< endl; 
  
 for(int i = 0; i < (sizeTwo - 1); i++) 
  {   
   alphaArray[i];  
  
  for(int j = 0; j < (sizeOne -1); j++) 
   {  
     buffArray[j];     
  
   if(alphaArray[i] == buffArray[j] ) 
   {  
    countFreq = countFreq + 1;  
   }  
   else 
   { 
    countFreq = 0; 
   }   
cout << "It's a match.  " << alphaArray[i] << "   shows up   " << countFreq << "  times." << endl << endl;       
     } 
    } 
 }  // end "parseBuffer"

Hi. I don't see where the cout statement uses the j variable, just the i one. It should be okay to make the cout statement the last line in the i loop.

Originally Posted by iMalc

If you don't want it done repeatedly then it should not be inside the loop. Just move it outside, down to line 48.

Hi iMalc,

Thanks for the response....the "cout" statement isn't working correctly after the "if" statement or the "else" statement.
If it's in the "if" section, it only outputs matches, however, each time it finds a match instead of the total.
If it's in the "else" statement, it outputs the "cout" statement every time only also with the -0- matches.

Originally Posted by whiteflags

Hi. I don't see where the cout statement uses the j variable, just the i one. It should be okay to make the cout statement the last line in the i loop.

Hi whiteflags,

Yes, I feel that surrendering feeling...code that doesn't work can make you feel like that...; )
I tried your suggestion and put the "cout" statement in the "i" loop, and it outputs the "cout" statement every time only also with the -0- matches.
You're right about the "j" loop, I won't be outputting anything in relation to it, it's the alphabet array used to compare to the User input. Please let me know if you have any other suggestions, thx!

Originally Posted by codechick

Please point out where my error is, thanks in advance!

<<My code is correctly counting the number of duplicates (or not)... >>

Well darling, it's not. You have a few errors here. I will clue you in on the errors.

Code:

for(int i = 0; i < (sizeTwo - 1); i++) 
  {   
   alphaArray[i];  
  
  for(int j = 0; j < (sizeOne -1); j++) 
   {  
     buffArray[j];     
  
   if(alphaArray[i] == buffArray[j] ) 
   {                                         <-------Don't need bracket  
    countFreq = countFreq + 1;  
   }                                         <-------Don't need bracket 
   else 
   {                                         <-------Don't need bracket
    countFreq = 0; 
   }                                         <-------Don't need bracket  
cout << "It's a match.  " << alphaArray[i] << "    shows up   " << countFreq << "  times." << endl  << endl;       
     } 
    } 
 }  // end "parseBuffer"

The brackets aren't errors, but they just make the code less clear:

Code:

  if(alphaArray[i] == buffArray[j] ) 
       countFreq = countFreq + 1;  
  else 
       countFreq = 0;         <-------- here ??

So countFreq keeps incrementing with each iteration where there is a match, but resets to zero when there is no match? You then lose all of your tally!

Code:

for(outer loop)
    {
      alphaArray[i];
        
       for(inner loop)
           { 
            buffArray[j];     
  
            if(alphaArray[i] == buffArray[j] ) 
                countFreq = countFreq + 1;
           }
    {

The outer loop sets up the first letter of the alphabet - a. Then it goes to the inner loop and compares that "a" to all of the letters in the buffer. So each iteration of the inner loop compares "a" with one letter in the buffer, one for each iteration, and each time a match is found countFreq is incremented. But what if there was no match for "a" in the buffer, are you going to print "It's a match"? Surely you have to determine if a match occurred or not before you print anything out, right?

After the inner loop has compared "a" to all of the characters in the buffer, then the loop ends. Then you can print out "It's a match" if a match actually occurred. Also, somewhere along the way, countFreq will have to be reset back to zero to begin anew.

Oh, and since there are 26 letters in the alphabet, shouldn't you have a loop somewhere that loops 26 times? I don't see that in your code. Rewrite your code and see how well it works. If you have anymore problems, re post your program and tell us what errors you are getting. Also, you did mention that if you moved the cout statement outside the loop then i and j would be out of scope. In that case, you could have defined i and j outside of the outer loop so that it would be in scope for the rest of the program, like so:

Code:

int i = 0;
int j = 0;

for(; i < 10; i++)
    {
       for(; j < 10; j++)
           {
             .....
             .....
           }

     }
cout << i << j;

Hello all, thank you big time for all of your feedback...; ) !!
I just got my PC back from the shop today and got your posts.
Let me look at it all of your tips carefully, and follow up shortly.
Thanks for giving the techie luv.

Just for starters, lines 34 and 38 do nothing, so you can remove them with no effect.
When you declare i and j outside the two for loops, you don't need to define them again inside the for loops. Doing so undoes putting them outside. But don't worry, you don't need to do that anyway. But if you WERE going to do that, then your for loop would look like this: for(; i < (sizeTwo - 1); i++). Notice how the declaration of i is missing from the for loop.

Concerning your cout statement: the inner for loop compares a single alphabet character to all of the buffer characters and keeps count of how many matches, if any, there were. After that happens, then use cout. In other words, the alphabet character "a" is compared to each element of the buffer and counts how many matches occurred, then the inner loop ends. That's when you print out the results. Oh wait..... what if there were no matches for "a"? Do you still do cout? Of course not, you only print out when there are matches. So before you use cout, you need to determine if there were matches, if not, then skip cout.

Sorry I can't just spill the beans clearly, because you have to think. Understand clearly what that inner for loop is doing first. That's critical. It works like I said above. And I will give you one clue - get rid of that else statement and the resetting of countFreq. Resetting countFreq needs to go after cout, since you print it out, right? Also, since there are 26 letters in the alphabet, then surely you need to loop 26 times somewhere, right? I don't see that in your code.

Make changes in your code, and re post it when you have done that so we can see what you have.

Thank you for the input. For the sake of simplicity I have recoded it to be in main only. It still does not work correctly, I will paste in the output:

Code:

#include <iostream>
#include <iomanip>
#include <string>
#include <algorithm>
#include <Windows.h>

using namespace std;

int main() 
{
    char alphabetArray[]={'a','b','c','d','e','f','g','h','i','j','k',
      'l','m','n','o','p','q','r','s','t','u','v','w','x','y','z','\0'};
    char buffer[11];
    int countFreq = 0;
    
    cout << "Enter the string up to 10 letters." << endl;
    cin.get(buffer, sizeof(buffer) );
    
    // loop through alphabet array
    for(int i = 0; i < sizeof(alphabetArray); i++)
    {
       // loop through buffer array
       for(int j = 0; j < sizeof(buffer); j++)    
        {
         // compare first letter in alphabet array to all letters in buffer
         // if there's a match increment the count   
         if(alphabetArray[i] == buffer[j] )
           countFreq = countFreq + 1;
           cout << "this is the count" << countFreq << endl;  
           
        }
     }
     countFreq = 0;
    
    system("Pause");
    return 0;
    
}

-------------------------------------------
OUTPUT:
Enter the string up to 10 letters.
bbbbccccdd
this is the count0
this is the count0
this is the count0
this is the count0
this is the count0
this is the count0
this is the count0
this is the count0
this is the count0
this is the count0
this is the count0
this is the count1
this is the count2
this is the count3
this is the count4
this is the count4
this is the count4
this is the count4
this is the count4
this is the count4
this is the count4
this is the count4
this is the count4
this is the count4
this is the count4
this is the count4
this is the count5
this is the count6
this is the count7
this is the count8
this is the count8
this is the count8
this is the count8
this is the count8
this is the count8
this is the count8
this is the count8
this is the count8
this is the count8
this is the count8
this is the count8
this is the count9
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count10
this is the count11
Press any key to continue . . .

Well, I'll be a son of a gun! You're getting closer! I think you are only two small adjustments away from getting this to work.

This statement: sizeof(alphabetArray) took care of looping the 26 times so that every letter of the aphabet gets compared to the buffer.

The only thing the outer for loop does is select another alphabet character for the inner loop to compare with every character in the buffer. Again, the solution to this problem lies in your clear understanding of the operation of the inner for loop.

OK, so here is the inner for loop:

Code:

for(int j = 0; j < sizeof(buffer); j++)

    {
      if(alphabetArray[i] == buffer[j] )
         ++countFreq;
    }

Now, when the program starts, the outer loop provides us with the letter "a" from the alphabet. Then we move into the inner loop to compare "a" with every character in the buffer. One loop compares "a" with the first char of buffer. If the buffer size is 10, then we have to loop 10 times to compare "a" with the whole buffer to see how many times "a" matches. Surely you wait until all 10 iterations occur before you cout anything, right? In other words, it takes 10 loops to find out how many "a" there are in the buffer, then another 10 loops to find out how many "b" there are in the buffer, and 10 more loops for "c" ,etc, etc. So then, where do you place your cout statement? Think, O wise one!

After you count how many "a" there are, you need to reset countFreq for the "b" comparison and then after that reset it for the "c" comparison, etc. And you know that you have to print out countFreq, so logically after cout you no longer need the value in countFreq. So maybe then is a good time to reset it? In other words, after the end of each 10 iterations, you have to reset countFreq for then next set of 10 iterations.

Also, you don't want to cout anything if there was no match. If "a" was compared to all 10 characters in the buffer with 10 loops, and there was no match, then you don't want to print out anything. But if there was a match, say 4 matches, then you would print that out.

Also, why not restore your original cout statement, that was good.

Fix your code and re post it if your program doesn't work. And of course, let us know if it does! : )

You are almost there. Like I said, it depends on how well you grasp what is going on in your inner and outer loop!

Hello Coder Head,

Thank you for sticking with me through the journey.
You gave me encouragement I was on the right track, and just needed a couple of key adjustments. This helped me to be CONFIDENT that I could figure it out with what I have.
So, here is the raw, and WORKING version of the code.
If there's anything else, please advise. Else I will post a final, full-on version with a function. Enjoy the code and output!

Code:

#include <iostream>
#include <iomanip>
#include <string>
#include <algorithm>
#include <Windows.h>

using namespace std;

int main() 
{
    char alphabetArray[]={'a','b','c','d','e','f','g','h','i','j','k',
      'l','m','n','o','p','q','r','s','t','u','v','w','x','y','z','\0'};
    char buffer[11];
        
    cout << "Enter the string up to 10 letters." << endl;
    cin.get(buffer, sizeof(buffer) );
    int countFreq = 0;
    
   // loop through alphabet array
    for(int i = 0; i < sizeof(alphabetArray); )
    {
     cout << "The letter " << alphabetArray[i] << "  matched " << countFreq << " times." << endl; 
     i++;
     countFreq = 0; 
           
      for(int j = 0; j < sizeof(buffer); j++)
       {    
        if( alphabetArray[i] == buffer[j] )
           countFreq = countFreq + 1;
       }
    }
    
    system("Pause");
    return 0;
    
}

---------------------------------------
OUTPUT:
Enter the string up to 10 letters.
bbbbccccdd
The letter a matched 0 times.
The letter b matched 4 times.
The letter c matched 4 times.
The letter d matched 2 times.
The letter e matched 0 times.
The letter f matched 0 times.
The letter g matched 0 times.
The letter h matched 0 times.
The letter i matched 0 times.
The letter j matched 0 times.
The letter k matched 0 times.
The letter l matched 0 times.
The letter m matched 0 times.
The letter n matched 0 times.
The letter o matched 0 times.
The letter p matched 0 times.
The letter q matched 0 times.
The letter r matched 0 times.
The letter s matched 0 times.
The letter t matched 0 times.
The letter u matched 0 times.
The letter v matched 0 times.
The letter w matched 0 times.
The letter x matched 0 times.
The letter y matched 0 times.
The letter z matched 0 times.
The letter matched 1 times.
Press any key to continue . . .