Word Occurance Problem: Output is wrong

[singinangel07]

Okay so I was the one with the Assertion failure problem earlier. So, I started all over. I don't have any compiling errors or any problem running the program, but now the output is wrong. My new code is:

#include <iostream>
#include <cstring>
using namespace std;

int main()
{
	char phrase[100]; //100 characters allowed 
	char *words[20]; //up to 20 words
	char tok_vals[] = " /n/t";
	int counter = 0;
	char *token;
	char *next_token;

	cout<< "Please enter a phrase: " << endl;
	cin.getline(phrase,100);

	for (int i = 0; i < 20; i++)
	{
		words[i]= "";// initializes the words array with blank strings
	}
	token = strtok_s(phrase, tok_vals, &next_token); //splits phrase into tokens

	while(token != NULL)
	{
		words[counter] = token; //while there are tokens in "string"
		counter ++;

		token = strtok_s(NULL, tok_vals, &next_token); // get the next token
	}
int count;
//print the occurances of each word
for(int i = 0; i < counter; i++)
{ 
	count = 1;
	
	for(int j = (i + 1); j < counter; j++)
	{
		if(strcmp(words[i],words[j]) == 0)
		{
				count++;
				words[j] = "";
		}
		if(words[i] != "")
		{
		     cout << words[i] << "-" << count << "\n";
		}
	}
}

return 0;
}

When the phrase "i am sam" is input into the program, the output is:
i-1
i-1
am-1

I'm not sure of what I need to change. Any help would be appreciated. Thank you for your time.

[algaes]

What everyone wants to know is what output you are looking to achieve? You are getting what the program tells you, but what do you expect (want) to see?

[Paul McKenzie]

Okay so I was the one with the Assertion failure problem earlier. So, I started all over. I don't have any compiling errors or any problem running the program, but now the output is wrong.

Have you used the debugger? If so, what have you observed that didn't do what it was supposed to do? If you haven't used the debugger, please do so.

One suggestion:

#include <string>
//...
std::string words[20]; //up to 20 words
//...
if( words[i] ==  words[j] )

Instead of storing pointers, you should be storing the actual words found. The goal of the word array is to store words. Pointers are not words (strings). The remaining code stays the same, and is easier to debug if there is a problem. By using pointers for the words array, you are risking mismanaging that array. What if that pointer no longer points to the place you expect it to, or that pointer no longer represents the original word that you found?

Instead, just store the word as-is (which is why it is a true string type, not a pointer). Then you don't risk any of what I mentioned above.

Regards,

Paul McKenzie

[alanjhd08]

Hi,

That's a lot closer to what you need!

All you need to do is move that final step where you print out the result out of the inner loop (for j) so that it is done once for each i.

So

for(int i = 0; i < counter; i++)
{ 
    count = 1;
	
    for(int j = (i + 1); j < counter; j++)
    {
        if(strcmp(words[i],words[j]) == 0)
        {
            count++;
            words[j] = "";
        }
    }
    if(words[i] != "")
    {
        cout << words[i] << "-" << count << "\n";
    }
}

[GCDEF]

words[i]= "";// initializes the words array with blank strings

That's not really what that does. words[i] = NULL would be more appropriate.

You can't compare null terminates strings using == and != because those operators look at the pointers, not the contents of the string. You need to learn the difference between char, char[] and char*.

[PredicateNormative]

Also, change

char tok_vals[] = " /n/t";

to

char tok_vals[] = " \n\t";

[singinangel07]

Thank you guys so much!! Problem fixed. :-) Paul and Alan, you guys are my saviors!

[PredicateNormative]

singinangel07, is this a homework question, or are you just doing it for fun?

[PredicateNormative]

I just read your homework requirements from your other post, and since the solution I have in mind fails the requirements (i.e. it doesn't use the required C functions), I don't have a problem with posting it, since you will not have the temptation to steal it. Anyhow, there is a safer "more C++" way of solving the problem than resorting to C functions. If you are interested, I could post the C++ solution when I get back from lunch assuming no one objects. The solution uses standard algorithms, STL containers and iterators, .

Does anyone think it would be wrong for me to post the solution described above to a homework question even though it fails the requirements? If so then I will not post it. I'll give 30 minutes for a response.

I make the bold highlight, because I want to catch peoples attention

[PredicateNormative]

#include <iostream>
#include <sstream>
#include <string>
#include <vector>
#include <iterator>
#include <algorithm> 
#include <cctype>     
#include <functional> 

class Disallowed : public std::unary_function<char, bool>
{
public:
  Disallowed(const std::string& tokens) :tokens_(tokens) {}

  bool operator()(char value) 
  { 
    return (std::string::npos != tokens_.find(value)); 
  }

private:
  std::string tokens_;
};

int main()
{
  std::string phrase;
  std::stringstream ss;

  std::cout << "Please enter a phrase:" << std::endl;
  std::getline(std::cin, phrase);

  replace_if( phrase.begin(), phrase.end(), Disallowed("!\"()<>,.?"), ' ');
  std::transform( phrase.begin(), phrase.end(), phrase.begin(), tolower);

  ss << phrase;

  std::vector<std::string> words(std::istream_iterator<std::string>(ss), (std::istream_iterator<std::string>()));
  std::sort( words.begin(), words.end() );

  std::vector<std::string> unique_occurances(words.size());
  std::vector<std::string>(unique_occurances.begin(), std::unique_copy(words.begin(), words.end(), unique_occurances.begin())).swap(unique_occurances);

  std::cout << std::endl << "Breakdown of that phrase:" << std::endl;

  for(std::vector<std::string>::iterator it=unique_occurances.begin(); it!=unique_occurances.end(); it++)
  {
    int count = std::count( words.begin(), words.end(), *it);
    std::cout << count << " occurances of " << *it << std::endl;
  }

  system("PAUSE");
  return 0;

}

[Paul McKenzie]

Does anyone think it would be wrong for me to post the solution described above to a homework question even though it fails the requirements? If so then I will not post it. I'll give 30 minutes for a response.

I make the bold highlight, because I want to catch peoples attention

I've done this a few times, provided what I've written is very much different than what was expected from the student. The responsibility is on the student if they just take your solution as-is and hand it in.

Either they will understand your solution and explain it when asked by the instructor what all of it does, or get in a whole heap of trouble (maybe even expulsion or suspension from school due to plaigarism) if they can't explain what it is they handed in.

One thing though, you should have left very little, if any comments in the code. This ensures that the poster who wants to understand it will actually do the research, instead of just repeating what you wrote in the comments.

Regards,

Paul McKenzie

[laserlight]

Personally, I would replace the use of the vectors with a map:

std::stringstream ss(phrase);

typedef std::map<std::string, std::size_t> WordFrequencyMap;
WordFrequencyMap word_frequencies;
std::string word;
while (ss >> word)
{
    ++word_frequencies[word];
}

std::cout << std::endl << "Breakdown of that phrase:" << std::endl;
for (WordFrequencyMap::iterator it = word_frequencies.begin(), end = word_frequencies.end();
    it != end; ++it)
{
    std::cout << it->second << " occurrences of " << it->first << std::endl;
}

[PredicateNormative]

I've done this a few times, provided what I've written is very much different than what was expected from the student. The responsibility is on the student if they just take your solution as-is and hand it in.

Either they will understand your solution and explain it when asked by the instructor what all of it does, or get in a whole heap of trouble (maybe even expulsion or suspension from school due to plaigarism) if they can't explain what it is they handed in.

One thing though, you should have left very little, if any comments in the code. This ensures that the poster who wants to understand it will actually do the research, instead of just repeating what you wrote in the comments.

Regards,

Paul McKenzie

Thanks Paul, assuming that the OP has not seen the code yet, I'll remove the comments.

[PredicateNormative]

Personally, I would replace the use of the vectors with a map:

std::stringstream ss(phrase);

typedef std::map<std::string, std::size_t> WordFrequencyMap;
WordFrequencyMap word_frequencies;
std::string word;
while (ss >> word)
{
    ++word_frequencies[word];
}

std::cout << std::endl << "Breakdown of that phrase:" << std::endl;
for (WordFrequencyMap::iterator it = word_frequencies.begin(), end = word_frequencies.end();
    it != end; ++it)
{
    std::cout << it->second << " occurrences of " << it->first << std::endl;
}

That's a nice simplification!