class implementation question

Okay so I feel like I almost have this correct but I just cannot get there. It is supposed to output a list with the number of occurences of each face and a histogram with the number of occurences and I just cannot figure out what I am missing.
The aDie class simply generates a number between 1-6 simulating a dice roll.

Am i missing something that I am just not seeing?

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
 #ifndef AHISTOGRAM_H
#define AHISTOGRAM_H
#include <iostream>
#include <vector>
using namespace std;

class aHistogram : public aDie
{
public:
	aHistogram(); // default constructor
	void update(int face); // update histogram counts
	void display(int maxLengthOfLine); // displays histogram
	void clear(); // clears counts
	vector<int> graph;
	
private:
	int face;
	int maxLengthOfLine;
};
#endif 

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
#include <iostream>
#include <cstdlib>
#include <ctime>
#include <vector>
#include "aDie.h"
#include "aHistogram.h"
using namespace std;

aHistogram::aHistogram() // default constructor
{
	face = ((rand() % 6) + 1);
	maxLengthOfLine = 60;
	graph.resize(6);
}
void aHistogram::update(int face) // updates roll count
{
	this->face = face++;
}

void aHistogram::display(int maxLengthOfLine) 
{
	int i;
	
	cout << "Face Appearances" << endl; // displays the appearances of each face
	cout << endl;
	for (i = 0; i < graph.size(); ++i)
	{
		cout << i + 1 << "---" << graph.at(i) << endl;
	}
	cout << endl;

	cout << "Histogram" << endl; // displays the histogram count
	cout << endl;
	for (i = 0; i < graph.size(); ++i)
	{
		cout << i + 1 << " - ";
		while (graph.at(i) > maxLengthOfLine)
		{
			cout << "X";
			graph.at(i) -= maxLengthOfLine;
		}
		cout << endl;
	}
	
}

void aHistogram::clear() // clears the counts for the appearances of each face 
{
	face = 0;
	maxLengthOfLine = 0;

}


1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
#include <iostream>
#include <cstdlib>
#include <ctime>
#include <vector>
#include "aDie.h"
#include "aHistogram.h"
using namespace std;

int main()
{
	int i;
	srand(time(0)); // seeding RNG
	
	aDie* Die1;
	aHistogram* Die2;
	vector<aDie*> DiceRoll(12000);

	Die1 = new aDie;
	Die2 = new aHistogram;

	for (i = 0; i < DiceRoll.size(); ++i) //rolls die and updates 12000 times
	{
		Die2->Roll();
		Die2->update(6);
	}
	
	Die2->display(60);

	cin.ignore();

	return 0;
}
ahistogram.h
--------------
line 7: There is no reason for aHistogram to inherit aDie. The relationship does not pass the "is a" test. A histogram is NOT a die.

Line 14: You don't need a 12,000 entry vector here. A simple array of 6 ints is all that is needed.
 
  int occurrances[6];


Line 16: Why do you have a vector here? You don't use (or need) it.

Line 17: Why is face a member? A histogram doesn't have a face.

ahistogram.cpp
----------------
line 11: What is the purpose of calculating face here? A histogram does not have a face.

Lines 24-30: Do you really need to display all 12,000 rolls? If so, a better place to do this would be in your loop in main.

Lines 32-43: You're trying to use your histogram for two different purposes. Lines 26-29, you're trying to display the value of all 12,000 rolls. Here, you're trying to display how many times each face is rolled. Those are two different things and you should not be using the same vector to do both.

main.cpp
---------
Lines 14-15: There is no reason for aDie and aHistogram to by dynamic. You have a memoy leak because to don't release them.

Line 15: Don't call your histogram Die2. It's a histogram, not a die.

Line 16: There is no reason to create a 12000 entry vector. What you're trying to measure is how many times each of the faces (1-6) is rolled. A simple array of 6 ints will suffice.

Line 18: What is the purpose of die1? You don't use it.

Your aDie::Roll function (from other thread) needs to return the face (value of the roll).

Line 23-24: This should be:
1
2
3
4
 
  int face; 
  face = aDie.Roll();
  histogram.update (face);  


Topic archived. No new replies allowed.