Histogram Help!

[mickeraiello]

This program analyzes an array of numbers. Everything in the program works, so don't worry about any other functions. I got to the histogram part, and found a solution but it had over 200 lines of code. There must be an easier way to do this, but I cant seem to find it. Anything will help. In this code I have the sloppy and lengthy bit of code. If anyone can give me help on finding a shorter and easier way, that would be great. I'm also a beginner programmer, so anything else you spot will be of help. Thanks in advance

  /*********************************************************************************************
* Problem Set Arrays Analyization.cpp : Defines the entry point for the console application. *
*                                                                                             *
* This program analyzes a set of numbers by the user entering numbers 1-50 and terminating   *
* with a sentinel. The program will then find the average, max, range, mode, of the numbers  *
* and also make a histogram                                                                  *
*                                                                                             *
* Author: Mike Aiello                                                                         *
* Date: December 19, 2017                                                                     *
* Freshman Masuk Highschool                                                                     *
**********************************************************************************************/

#include "stdafx.h"
#include <iostream>

using namespace std;

/********************************************
* This function calcuates the average number
* for the array of numbers
*
* Pre: array,and counter must be integers
* Post: averageNumber is returned as an integer
*********************************************/
void AverageNumber(int setOfNumbers[], int &averageNumber, int counter)
{
	int sum = 0;

	for (int i = 0; i<counter; i++)
	{
		sum += setOfNumbers[i];

	}
	averageNumber = sum / counter;

}

/********************************************
* This function calcuates the max/highest number
* in the array of numbers
*
* Pre: array,and counter must be integers
* Post: maxNumber is returned as an integer
*********************************************/
void MaxNumber(int setOfNumbers[], int &maxNumber, int counter)
{
	int max = 0;

	for (int i = 0; i<counter; i++)
	{
		if (setOfNumbers[i]>max)
		{
			max = setOfNumbers[i];
		}
	}
	maxNumber = max;
}

/********************************************
* This function calcuates the range of numbers
* whcih means the highest and lowest numbers
*
* Pre: array,and counter must be integers
* Post: leastNumber is returned as an integer
*********************************************/
void Range(int setOfNumbers[], int &maxNumber, int &leastNumber, int counter)
{
	int max = 0;
	int least = 50;

	for (int i = 0; i<counter; i++)
	{
		if (setOfNumbers[i]>max)
		{
			max = setOfNumbers[i];
		}
	}

	maxNumber = max;

	for (int j = 0; j<counter; j++)
	{
		if (setOfNumbers[j]<least)
		{
			least = setOfNumbers[j];
		}
	}

	leastNumber = least;
}

/********************************************
* This function calcuates the mode of the numbers
* in the array
*
* Pre: arrays,and counter must be integers
* Post: mode is returned
*********************************************/
void Mode(int setOfNumbers[], int &mode, int occurences[], int counter)
{

	for (int k = 1; k <= 50; k++)
	{
		if (occurences[k]>mode)
		{
			mode = k;
		}
	}


}

/********************************************
* This function displays a histogram of the
* numbers
*
* Pre: arrays,and counter must be integers
* Post: none
*********************************************/
void Histogram(int setOfNumbers[], int occurences[], int counter)
{
	int numberInRange = 0;

	cout << "1-5";
	for (int i = 1; i <= 5; i++)
	{
		occurences[i] += numberInRange;
	}
	for (int i = 1; i <= numberInRange; i++)
	{
		cout << "*";
	}
	cout << endl;
	numberInRange = 0;

	
	cout << "6-10";
	for (int i = 6; i <= 10; i++)
	{
		occurences[i] += numberInRange;
	}
	for (int i = 1; i <= numberInRange; i++)
	{
		cout << "*";
	}
	cout << endl;
	numberInRange = 0;


	cout << "11-15";
	for (int i =11; i <= 15; i++)
	{
		occurences[i] += numberInRange;
	}
	for (int i = 1; i <= numberInRange; i++)
	{
		cout << "*";
	}
	cout << endl;
	numberInRange = 0;

	//so on adding 5 to the range each time
}


int _tmain(int argc, _TCHAR* argv[])
{
	int setOfNumbers[100];
	int occurences[51];

	int userInput;
	int counter = 0;

	int averageNumber;
	int maxNumber;
	int leastNumber;
	int mode;

	do
	{
		cout << "Enter a number 1-50 (Terminate with -1)" << endl;
		cin >> userInput;

		if (userInput >= 1 && userInput <= 50)
		{
			setOfNumbers[counter] = userInput;
			counter++;
		}
		else if (userInput == -1)
		{
			cout << "Terminating" << endl;
		}
		else
		{
			cout << "1-50 Please!!" << endl;
			system("pause");
		}

		system("cls");

	} while (userInput != -1);

	for (int i = 1; i <= 50; i++)
	{
		occurences[i] = 0;
	}

	for (int i = 0; i<counter; i++)
	{
		for (int j = 1; j <= 50; j++)
		{
			if (setOfNumbers[i] == j)
			{
				occurences[j] = occurences[j] + 1;
			}
		}
	}

	cout << "Numbers: ";

	for (int i = 0; i<counter; i++)
	{
		cout << setOfNumbers[i] << " ";
	}



	cout << endl << endl;

	AverageNumber(setOfNumbers, averageNumber, counter);
	cout << "Average Number = " << averageNumber << endl;

	MaxNumber(setOfNumbers, maxNumber, counter);
	cout << "Max Number = " << maxNumber << endl;

	Range(setOfNumbers, maxNumber, leastNumber, counter);
	cout << "Range = " << leastNumber << "-" << maxNumber << endl;

	Mode(setOfNumbers, mode, occurences, counter);
	cout << "Mode = " << mode << endl;

	Histogram(setOfNumbers,occurences,counter);

	system("pause");
	return 0;
}

[dhayden]

AverageNumber() will fail if the user enters numbers 1 and 2. Remember: the average of a bunch of integers isn't always an integer.

Range() duplicates a bunch of code in MaxNumber(). Why not create a MinNumber() function and then just call MaxNumber() and MinNumber() to get the range?

//so on adding 5 to the range each time
Any time you find yourself repeating the same code over and over with minimal changes, it's a good sign that you can factor out some of the code.

Think about the histogram. You should start by deciding how many bars are in the histogram, then your code should be something like:

for (barNumber=0; barNumber<numBars; ++barNumber) {
    // compute the lowest value that goes in this bar
    // compute high value that goes in this bar.
    // count up the number of occurences between the lowest and highest values.
    // This is the height of the bar.

    // print the bar
}

[Duthomhas]

<Quick glance>
It looks well-organized and nicely-done!

This is more-or-less what is expected of you when you write the code for a beginning course. Having library functions do it for you does not help with the understanding.

That said, C++ makes life easy by providing a lot of functionality out-of-the-box, as it were. A std::map<> makes for a fabulous histogram. And various <algorithm> functions exist that can help with the other things.

Here is (a slightly different) version of your program using C++ stuff, heavily commented for your perusal.

#include <algorithm>  // max_element(), minmax_element(), max(), count_if()
#include <ciso646>    // and or
#include <iostream>   // cin, cout
#include <limits>     // numeric_limits<>
#include <map>        // map<>
#include <numeric>    // accumulate()
#include <sstream>    // istringstream
#include <string>     // string
#include <vector>     // vector

int main()
{
  // This is our list of integer numbers.
  std::vector <int> xs;
  {
    // Instruct the user and get input as a STRING.
    std::cout << "Enter a bunch of numbers with value in [1,50], separated by spaces.\n";
    std::string s;
    getline( std::cin, s );

    // Now we'll convert the string to the list of numbers, checking along the way that
    // the user only inputs INTEGER numbers in the range [1,50]. If the user fails to
    // input an integer, the stream state will fail automatically. Else if the number is
    // out of range, we fail the stream ourselves.
    std::istringstream ss{ s };
    int x;
    while (ss >> x)
      if ((1 <= x) and (x <= 50)) xs.push_back( x );
      else                        ss.setstate( std::ios::failbit );

    // Either way, if we didn't read the ENTIRE stream, something went wrong.
    // Tell the user and make him (or her) start over.
    if (!ss.eof())
    {
      std::cout << "One of those things is not a number in [1,50].\nFooey.\n";
      return 1;
    }
  }

  // Great. Now to print our list of numbers.
  std::cout << "Numbers:";
  for (int x : xs) std::cout << " " << x;
  std::cout << "\n";

  // The average is the sum of all elements divided by the cardinality.
  // We use two functions to help us:
  //   accumulate() sums all the numbers
  //   max() makes sure we don't try to divide by zero (in case the user entered nothing)
  // (We could just say "None" like below, but this shows something useful.)
  std::cout << "Average Number = "
    << std::accumulate( xs.begin(), xs.end(), 0 ) / std::max( 1ULL, xs.size() )
    << "\n";

  // Finding the maximum value in a list is also a function in in C++.
  // We just need to be careful again about the possiblity of an empty list.
  std::cout << "Max Number = ";
  if (xs.size()) std::cout << *std::max_element( xs.begin(), xs.end() ) << "\n";
  else           std::cout << "None\n";

  // The range is a single number!
  // It is the difference between the largest and the smallest values in the array.
  // Here is another function, this time it finds both min and max at the same time.
  // (We could have used it above and avoided finding the max twice.)
  std::cout << "Range = ";
  auto mm = std::minmax_element( xs.begin(), xs.end() );
  if (xs.size()) std::cout << (*mm.second - *mm.first) << "\n";
  else           std::cout << "None\n";

  // The mode is the number that appears most often in the list.
  // In order to find it, we first need a histogram.
  // A HISTOGRAM is a list of pairs. Each pair is composed of two parts:
  //   * an x value (from xs)
  //   * a count (the number of times x appears in xs)
  // To build it we simply loop through xs and increment the count for every x
  // we find. (Counts start at zero.)
  std::map <int, unsigned> histogram;
  for (int x : xs) histogram[ x ] += 1;

  // Since the mode only exists if there is a SINGLE element that occurs most,
  // we need to code against the possibility that it does not exist.

  // First, we find ANY element with the largest count.
  auto max0 = std::max_element( histogram.begin(), histogram.end(),
    // This here magic is a LAMBDA -- it is a function, just without a name.
    // It takes two (x, count) pairs and compares their counts. The max_element()
    // function uses it to find the largest pair (the one with the largest count).
    []( auto a, auto b ) { return a.second < b.second; } );

  // Now we want to know how many pairs in our histogram have the same count as
  // the element we just found.
  auto n = std::count_if( histogram.begin(), histogram.end(),
    // Again we use a LAMBDA -- an unnamed function that takes a pair and
    // returns whether or not that pair's count is the same as max0's count.
    [&max0]( auto p ) { return p.second == max0->second; } );

  // Finally, we can print our result, but only if the maximum count is unique.
  if (n == 1) std::cout << "Mode = " << max0->first << "\n";
  else        std::cout << "No Mode\n";

  // Printing the histogram is very much like printing xs: loop through
  // all pairs and print both parts (x, count).
  std::cout << "Histogram:\n";
  for (auto p : histogram)
    std::cout << "Value = " << p.first << " : " << p.second << " time(s)\n";

  // To keep the terminal open, use something like the following.
  //  * The next line is commented out because we used getline() to read a string
  //    as our last input into the program.
  //  * If we had used something like "std::cin >> foo" as our last input, then
  //    we would need to uncomment the line in order to get rid of the unread
  //    newline sitting in the input buffer.
  //std::cin.ignore( std::numeric_limits <std::streamsize> ::max(), '\n' );

  // Finally, give the user instruction and wait.
  std::cout << "Press Enter to quit...";
  std::cin.get();
}

This is just to show you the power of modern C++, and how easy it is to do things in just a few lines of code (not counting excessive commentary, of course).

As you continue your learning you will become more familiar with stuff like this and your programs will become shorter and more powerful.

Hope this helps.

[mickeraiello]

@dhayden
Thanks so much I really appreciate it. I figured it out and did what you said. I know this isn't getting published but do you want me to add your name to it, just for I don't know, like citation I guess. Because some of it is your code. Anyways, I appreciate the help, I'm still learning so things don't come as easily to me. Thanks!!!!!!!!!!!!!!!!!!!

[mickeraiello]

@Duthomhas
Yea, I understand. This is my first year programming, and I've been doing it for about 4 months now. In class we have worked with libraries such as string, and ctime for random numbers. We haven't gotten to in depth, but what you showed me looks cool, and a lot easier and shorter. I cant wait till we start using more libraries like the ones you showed. Thanks for the help!!!!!