[nomonkeybusiness]

Hi, i'm new here, and new at c++ and i need help with a small program i'm making.
It is supposed to count the characters in a userinput-string.
(i.e: if the user enters "stuff", the statistics will show that there is 1 "s", 1 "t", 1 "u" and 2 "f"s)
My problem is that the program outputs:
1 s
1 t
1 u
2 f
2 f

Wich is wrong. The correct output should be:
1 s
1 t
1 u
2 f

here's my code (with a pre-set string):

int main(int argc, char** argv)
{
	std::string google = "google";

	std::vector<char> chars(google.size());
	std::vector<int> values(google.size());

	
	for(unsigned int i = 0; i != google.length(); i++)
	{
		chars.at(i) = google.at(i);
	}
	
	
	for(unsigned int i = 0; i != google.length(); i++)
	{
		for(unsigned int j = 0; j != google.length(); j++)
		{
			if(chars.at(i) == google.at(j))
			{
				values.at(i)++;
			}
		}
		
	}
	
	for(unsigned int i = 0; i != google.length(); i++)
	{
		std::cout << chars.at(i) << " occurs " << values.at(i) << " times " << std::endl;
				
	}
	
	return 0;
}

[tree6014]

The problem is that you were indexing every character you looked at even if it had already been indexed. There were some other small changes I made such as saying ++i rather than i++ for a minor efficiency reason. Also, it is generally considered to be bad form to declare variables part way through the code (in the for loops). Since you are using vectors, you might as well use their dynamic features because that is the point of vectors and it makes everything easier and less prone to error. Also, you can say "using namespace std;" after your includes so that you don't have to put "std::" in front of everything. I also changed "!=" to "<" not because "!=" is wrong so much as, in the event that i or j were somehow incremented more than 1, "<" will prevent an infinite loop from occurring. So, enough babbling, here is the code.

#include <iostream>
#include <vector>

using namespace std;

int main(int argc, char** argv)
{
	string google = "google";

	vector<char> chars;
	vector<int> values;

	unsigned int i;
	unsigned int j;

	bool indexed = false;
	
	for(i = 0; i < google.length(); ++i)
	{
		for(j = 0; j < chars.size(); ++j)
		{
			if(chars.at(j) == google.at(i))
				indexed = true;
		}
		if(!indexed)
		{
			chars.push_back(google.at(i));
			values.push_back(0);
		}
		indexed = false;
	}
	
	
	for(i = 0; i < chars.size(); ++i)
	{
		for(j = 0; j < google.length(); ++j)
		{
			if(chars.at(i) == google.at(j))
			{
				values.at(i)++;
			}
		}		
	}
	
	for(i = 0; i < chars.size(); ++i)
	{
		cout << chars.at(i) << " occurs " << values.at(i) << " times." << endl;				
	}
	
	return 0;
}

Hope this helps.

[NickDMax]

You know. Associative arrays work a little better than vectors in this case. Look at the Map object. By using a map<char, int> you can keep track of an integer paired with a unique character. Here is an example (a little fancy I know but I wanted to show as much STL as I could pack in there...).

#include <iostream>
#include <map>
#include <string>
#include <algorithm>
#include <cstdlib>
#include <iomanip>
using namespace std;

/**
 * CharHistogram is a functor class used to calculate a histogram of letters that occur in a word.
 * It is a functor so that it may be used with the STL for_each() algorithm.
 * The heart of the class is the map<char, int> which associates an integer value for each
 * character that gets inserted. This allows us to keep track of which characters have occured
 * as well as how many times the character occurs.
 */
class CharHistogram {
private:
	  //internal object to keep track of the character and how many times they happen.
	  // it pairs a unique character with an integer value that is used to "count" its occurances.
	  map<char, int> histogram;
public:
	//used to make this object a functor (an object that looks/acts like a function).
	// This operator will make the object behave like it is a function:
	//   CharHistogram cHist;
	//   cHist('a'); <- increments the value associated with the char 'a'.
	void operator()(char c) {
		//only care about alphabetic values
		if (isalpha(c)) {
			c = tolower(c);
			histogram[c]++;// = histogram[c]+1;
		}
	}
	
	//Function to display the contentse of this histogram.
	void displayStats() {
		map<char,int>::iterator it;
		cout << "There were " <<  histogram.size() << " unique letters.\nHistogram break down was:\n";
		for ( it=histogram.begin(); it != histogram.end(); it++ ) {
			cout << (*it).first << " occurs " << setw(3) << (*it).second << (((*it).second == 1) ? " time." : " times.") <<endl;
		}
	}
	
	//Function to display a little graph of the data...
	void displayRawHistogram() {
		map<char,int>::iterator it;
		for ( it=histogram.begin(); it != histogram.end(); it++ ) {
			cout << (*it).first  << " :";
			for (int i = 0; i < (*it).second; i++)  {
				cout << "*";
			}
			cout << endl;
		}
	}
};

int main() {
	
	// A string to test with...
	string testStr = "It was the best of times, it was the worst of times;" 
		   		   	 " it ws the age of wisdom, it was the age of foolishness;"
					 " it was the epoch of belief, it was the epoch of incredulity;"
					 " it was the season of Light, it was the season of Darkness;"
					 " it was the spring of hope, it was the winter of despair;"
					 " we had everything before us, we had nothing before us;"
					 " we were all going directly to Heaven, we were all going the other way";
	CharHistogram cHist;
	cHist = for_each(testStr.begin(), testStr.end(), cHist);
	cHist.displayStats();
	cout << "\nRaw histogram: \n";
	cHist.displayRawHistogram();
	system("pause");
	return 0;
}
	

Another note about the map object is that it sorts the keys as you insert them. So the list comes out nice and sorted.

This post has been edited by NickDMax: 16 October 2008 - 02:33 PM

[nomonkeybusiness]

NickDMax, on 16 Oct, 2008 - 02:30 PM, said:

You know. Associative arrays work a little better than vectors in this case. Look at the Map object. By using a map<char, int> you can keep track of an integer paired with a unique character. Here is an example (a little fancy I know but I wanted to show as much STL as I could pack in there...)

Heh, a bit fancy for my taste, but thank you anyway. I will sure try the map-object, seems intresting.


tree6014: You put me in the right direction. After some fiddeling i managed to get it to work. Thanks a lot.

ps. about the using-directive, i allready knew actually, but i prefer to typ std:: in front of my commands, partly because i think it looks nice, and partly because it feels more organized.