Regarding Histogram - confused

[Deadmau6]

I need help with my code, what i'm trying to accomplish is when the user using my program marks a student and gives them "X" amount of marks it'll print a star on the histogram e.g. 0-29 *

My program codes for starters

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

int main()
{
	std::map<char, int> hist_base;
	    char ch;

	    	    while (std::cin.get(ch))
	        ++hist_base[ch];

	    std::map<char, int>::const_iterator it = hist_base.begin();
	   std::map<char, int>::const_iterator end = hist_base.end();

	    
	   for (; it != end; ++it) {
	        if (std::isprint(it->first))
	            std::cout<< std::setw(2) <<'\''<< it->first <<'\'';
	        else
	            std::cout<< std::setw(4) << static_cast<int>(it->first);

	        std::cout<<": "<< std::string(it->second, '*') <<'\n';
	   }
	   return 0;
}

Anyway i can improve, any help would be apprieciated thanks.

[PanGalactic]

Would you mind editing your post and putting the source inside code tags? It will make it a lot more legible and folks here will be more likely to look at it.

[Deadmau6]

Thanks for that, i didn't know there was a option to do it.

[PanGalactic]

One thing that I find makes code like this more legible is liberal use of typedefs.

typedef std::map<char, int> histogram_type;
typedef histogram_type::const_iterator const_iterator;

histogram_type hist_base;
...
const_iterator it = hist_base.begin();
...

Another is to put the body of the for loop into a function and use std::for_each.

void write_hist(const histogram_type::value_type& value)
{
    if (std::isprint(value.first))
        std::cout<< std::setw(2) <<'\''<< value.first <<'\'';
    else
        std::cout<< std::setw(4) << static_cast<int>(value.first);

    std::cout<<": "<< std::string(value.second, '*') <<'\n';
}

...

std::for_each(hist_base.begin(), hist_base.end(), write_hist);

This eliminates the need for the const_iterator typedef mentioned above and the iterator variables themselves because the code is simplified to the point of not needing them.

I also recommend putting the code that reads the histogram data from std::cin into its own function.

histogram_type read_hist(std::istream& input)
{
    histogram_type hist_base;
    char ch;

    while (input.get(ch))
        ++hist_base[ch];

    return hist_base;
}

main() then looks like this:

int main()
{
    histogram_type hist_base = read_hist(std::cin);
    std::for_each(hist_base.begin(), hist_base.end(), write_hist);
    return 0;
}

And lastly, don't mix spaces and tabs for indentation. I recommend sticking with spaces for indentation. It looks the same for everyone.

[Deadmau6]

Thanks mate.