Question

This is an assignment in my advanced C++ class, we're going over pointers and dynamically allocated arrays, though that doesn't seem to be where my trouble lies...

Function header, Quote, Words, and Count are the important arrays, NumWords just holds a count of how many elements are in the Quote array.

void CountWords ( char ** Quote, char **& Words, int *& Count, int NumWords )
{

Variables and such, some of the arrays are being passed by pointer from main.

    char ** Temp = nullptr;
    int * ITemp = nullptr;
    int WordCount = 0;
    int QuoteCount = 0;

What follows is a priming read for the For loop coming up. I'm creating two dynamically allocated arrays, one to store a new found instances of a word in the Quote array, and another to store a count of how many times that word appeared in the Quote array. All this seems to be working fine, but I don't like how large everything is (Code Bloat) any advice to get this started differently would be excellent.

First dynamic array for char's.

Temp = new char * [WordCount + 1];
    for (int z = 0; z < WordCount; z++)
    {
        Temp[z] = Words[z];
    }
Temp[WordCount] = new char[ strlen( Quote[WordCount]) +1  ];
strcpy( Temp[WordCount], Quote[WordCount] );
delete [] Words;
Words = Temp;

Second dynamic array for int's.

ITemp = new int [ WordCount + 1 ];
    for (int z = 0; z < WordCount; z++)
    {
        ITemp[z] = Count[z];
    }
ITemp[WordCount] = 0;
delete [] Count;
Count = ITemp;

This for loop is supposed to use the new value in the **char array, iterate through Quote to find other instances of that value, and increments Count at the same index number for that element. Seems to work...

for (int j = 0; j < NumWords; j++)
{
    if (_stricmp( Words[ WordCount ], Quote[j]) == 0 )
    {
    Count[ WordCount ]++;
    }
}

Counters for how many words have been stored in the Words array, and where in the Quote array the function is.

WordCount++;
QuoteCount++;

This is where things start to go wonky...

for (int i = 0; i < NumWords; i++)
{

//Right here is where the program breaks, after the second iteration of this
//For-loop.  What happens is the loop counter (i) increments to 2 shortly 
//before Words[2] gets created.  I've tried decrementing i inside the If,
//where QuoteCount gets incremented, but that causes an out-of-bounds error
//on the Quote array on the last iteration.

//Check to see if Quote value is in Words
if (_stricmp( Quote[ QuoteCount ], Words[ i ]) == 0 )
{
    //If true, move on to next element in Quote array.
    QuoteCount++;
}

//If false, write Quote value to Words
else
{
        Temp = new char * [WordCount + 1];
        for (int z = 0; z < WordCount; z++)
        {
            Temp[z] = Words[z];
        }
        Temp[WordCount] = new char[ strlen( Quote[WordCount]) +1  ];
        strcpy( Temp[WordCount], Quote[WordCount] );
        delete [] Words;
        Words = Temp;

 //Create new array element in Count to track new value in Words
        ITemp = new int [ WordCount + 1 ];
        for (int z = 0; z < WordCount; z++)
        {
            ITemp[z] = Count[z];
        }
        ITemp[WordCount] = 0;
        delete [] Count;
        Count = ITemp;


//Check Quote for other instances of Word
        for (int j = 0; j < NumWords; j++)
        {
            if (_stricmp( Words[ WordCount ], Quote[j]) == 0 )
            {
            Count[ WordCount ]++;
            }//if
        }//for
//Increment word counter, indicating a new word was stored.

        WordCount++;

    }//else
}//for
}//function

I feel this has become far more complicated than it needs to be. I tried nesting For-loops to begin with, but I couldn't seem to get that working either. Another thing is, once it copies a word from Quote to Words, it shouldn't copy that word again.

Also, general input on code quality and such would be appricated. I'm striving to become a software engineer later in life, and I like to be the best at what I do, so I need to learn as much as I can all the time.

Was it helpful?

Solution

Your main issue is you seem to want to step through the quote using i, but at the same time you are using it to index Words. A more logical way to do things is to check each word of the quote against the existing words you have already copied into Words, and either incrementing the count or inserting the word accordingly. You can also take advantage of the fact that there will be at most NumWords in Words and allocate enough memory at the start of the function:

void CountWords(char const** Quote, char**& Words, int*& Count, int NumWords)
{
  Words = new char*[NumWords];
  Count = new int[NumWords];

  int words = 0;

  for (int i = 0; i < NumWords; ++i) {

    int j = 0;
    for (; j < words; ++j) {

      if (!strcmp(Quote[i], Words[j])) { // Duplicate word
        ++Count[j];
        break;
      }
    }

    if (j == words) { // New word found
      Words[words] = new char[strlen(Quote[i]) + 1]{};
      strcpy(Words[words], Quote[i]);
      Count[words] = 1;
      words++;
    }
  }

  Count[words] = 0;
}

int main()
{
  char const* quote[] = {"Hello", "world", "hello", "world"};
  char** words;
  int* count;
  CountWords(quote, words, count, 4);

  for (int i = 0; count[i]; ++i) {
    std::cout << words[i] << ' ' << count[i] << '\n';
    delete[] words[i];
  }

  delete[] words;
  delete[] count;      
}

In any case array / pointer shenanigans are hard to read and error-prone, there is hardly any code here (or your original code) that could be called "c++" aside from new, it's c with a dash of c++. The entire app can be more easily written in modern c++ like this:

#include <iostream>
#include <unordered_map>

int main()
{
  std::string word;
  std::unordered_map<std::string, int> map;

  while (std::cin >> word)
    ++map[word];

  for (auto const& w : map)
    std::cout << w.first << " : " << w.second << '\n';
}

And used like this: app < textfile.txt

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top