PPP2 Chapter 15 Exercise 6

[DragonOsman]

Exercise specification:

Design and implement a bar graph class. Its basic data is a vector<double>
holding N values, and each value should be represented by a “bar” that is
a rectangle where the height represents the value.

My code:

// Osman Zakir
// 7 / 4 / 2017
// Bjarne Stroustrup: Programming: Principles and Practice Using C++ 2nd Edition
// Chapter 15 Exercise 6
// chapter15ex6.cpp
// Exercise Specifications:
/**
* Design and implement a bar graph class. Its basic data is a vector<double>
* holding N values, and each value should be represented by a “bar” that is
* a rectangle where the height represents the value.
*/

#include <vector>
#include <iostream>
#include "../../Graph.h"
#include "../../Window.h"

namespace Graph_lib
{
	struct Bar_graph : Shape
	{
		Bar_graph(const std::vector<double> &values, const int N);
		~Bar_graph();
		void set_vector_size(const int size);
		void set_N(const int value) { m_N = value; }
		void draw_lines() const;
		const std::vector<double> &values_v() const { return m_values; }
		const std::vector<Rectangle *> &bars_v() const { return m_bars_v; }
		int N() const { return m_N; }
	private:
		std::vector<double> m_values;
		std::vector<Rectangle *> m_bars_v;
		int m_N;
	};
}

int main()
{

}

Graph_lib::Bar_graph::Bar_graph::Bar_graph(const std::vector<double> &values, const int N)
	:m_values{ values }, m_N{ N }
{
	if (N <= 0)
	{
		error("Invalid number of values!");
	}
	m_values.reserve(N);
	constexpr int width = 100;
	double bar_spaces = width * 1.0 / (2 * N - 1);
	for (int i = 0; i < N; ++i)
	{
		int height = std::round(values[i]);
		m_bars_v.push_back(new Rectangle{ Point{height / width, height}, width, height });
	}
}

Graph_lib::Bar_graph::~Bar_graph()
{
	for (auto x : m_bars_v)
	{
		delete x;
	}
}

void Graph_lib::Bar_graph::set_vector_size(const int size)
{
	m_N = size;
	m_values.reserve(m_N);
}

void Graph_lib::Bar_graph::draw_lines() const
{
	for (auto &x : m_bars_v)
	{
		x->draw();
	}
}

I'm wondering if my constructor code and everything is fine? And if not, please help. Also, I have a variable in the constructor that represents the spaces between bars, but I don't know how to use it.

Anyway, any help on this would be appreciated. Thanks in advance.

[dhayden]

You don't need m_N and shouldn't have it. Use m_values.size() to get the number of items. This is keeping with the general principal that any data item should exactly one, authoritative, representation. If you have two representation then you have to worry about keeping them consistent.

bars_v() method: who owns the points to the rectangles? How long are the pointed-at Rectangles valid? Who should delete them? The answers to all of these questions need to be crystal clear any time you return a pointer. That's why it's generally a bad idea to return pointers. So I'd make this a vector of Rectangles instead.

Right now you have to add all the values to the Bar_graph in the constructor. Yuck. What if someone wants to add them over time? Shouldn't that be possible? But if you create something like an add() method, then the class has to recompute the m_bars_v member. And if the caller adds a million data points, then your class has to compute all those bars, every single time. Yuck.

So rather than storing the m_bars_v vector all the time, I'd compute it inside bars_v() and have bars_v() return a vector rather than a reference to a vector.

I'm confused: draw_lines() draws rectangles? Then why not call it draw_rectangles()? I'd just call this method draw(). Sinceu m_bars_v won't be a member, draw() would be something like:

void Graph_lib::Bar_graph::draw()
{
    for (auto &rec : bars_v()) {
        rec->draw();
    }
}

[DragonOsman]

No, no, read the exercise specs, please. N has to be there. And draw_lines() is a virtual function declared in Graph.h (from Dr. Stroustrup's GUI interface library, which I'm using), same for draw().

Did you not read? If I'm doing it wrong, please at least tell me a way I can follow the exercise specs in a way that it's not "Yuck". But don't tell me to not follow the specs.

Again, here are the exercise specs:

Design and implement a bar graph class. Its basic data is a vector<double> holding N values, and each value should be represented by a “bar” that is a rectangle where the height represents the value.

And as it says in the comments in my code, this is an exercise for Dr. Stroustrup's book, Programming: Principles and Practice Using C++ 2nd Edition. Chapter 15 Exercise 6.

If you want me to, I could copy-paste the code from Graph.h, Graph.cpp, GUI.h and GUI.cpp in a later post?

Edit: I guess I should also add Window.h and Window.cpp in that list.

[dhayden]

No, no, read the exercise specs, please. N has to be there.

The spec you posted says that the class should use a vector that holds N values. Nowhere does it say that the class must store the value N as a separate member.

And draw_lines() is a virtual function declared in Graph.h (from Dr. Stroustrup's GUI interface library, which I'm using), same for draw().

Fair enough, but your original post didn't mention that draw_line() was virtual and didn't declare it that way. So how would I know?

don't tell me to not follow the specs.

There is nothing in my comment that violates the specs that you posted. If there are most specs, then please post them.

And as it says in the comments in my code, this is an exercise for Dr. Stroustrup's book, Programming: Principles and Practice Using C++ 2nd Edition. Chapter 15 Exercise 6.

Okay, but what if I don't have the book (I don't). All I have to go on is what you've posted.

please at least tell me a way I can follow the exercise specs in a way that it's not "Yuck".

I apologize if my "yuck" comment offended you. I didn't mean it in any mean way. I was trying to show the thought process that lead me to think that the design isn't optimal.

I could copy-paste the code from Graph.h, Graph.cpp, GUI.h and GUI.cpp

I don't think that's necessary. As I said, you might want to post the full text of the exercise. Meanwhile. Please consider my suggestions, other than the one about changing draw_line(), which you've pointed out needs to remain as-is.

[DragonOsman]

I'm not trying to say that your "Yuck" comment offended me (it didn't). I'm asking for a way to make it "not Yuck".

The exercise text is as I've posted it. I made N a separate variable to make it so that the user can specify ahead of time how many values the vector will hold so I can reserve that many values in the vector.

I can tell you don't have the book. That's why I said I'll post the GUI interface library code.

The function Bar_graph::draw_lines() overrides Shape::draw_lines(). Shape::draw_lines() is a virtual void function declared as virtual void draw_lines() const; in the namespace Graph_lib. Since mine overrides that virtual function, I'm not supposed to declare it as virtual. Right?

[dhayden]

I still say don't store the value N. Consider this:

vector<double> vec{0.0, 1.2}; // vector of 2 doubles
Graph_lib::Bar_graph b(vec, 30);  // crash! 

Your code will crash (or at least create undefined behavior) at line 54 because you'll attempt to access 30 elements in a vector that only contains 2.

I made N a separate variable to make it so that the user can specify ahead of time how many values the vector will hold so I can reserve that many values in the vector.

I'd make m_values public. That way users of the class can reserve the space themselves. They can also add, delete, or modify the values at will. This makes the class much more useful.

Do you need to specify the size and position of the bar graph? Otherwise how will you know where to draw it?

[jlb]

I agree that there is no need to store the value of 'N', I will go even farther to say that there is absolutely no need to know how many values will be stored in the vector before hand. In fact there is no need for anyone to know the value of 'N' before hand, you're using a vector so let the data determine the value of 'N', just push data into the vector and use the size of the vector, after the data entry is finished, to determine the value.

And note that that 'N' in the book is not a "normal" character but a character that is an italicized character in a different font, this usually denotes that the character has a special meaning. IMO, in this case it means that there will be an unknown number of data points that you will need to graph.

[DragonOsman]

The constructor for Rectangle comes in two or three flavors, and the one I use takes the top left point (the constructor for Point takes an x and a y as arguments) and the width and height as arguments. The exercise says to use Rectangles as the "bars" where the value is represented by the height, right? So I could just position the individual bars where I want them (I do need to how to use the bar_spaces variable I have in the constructor - any ideas?).

I also need some help on getting the top-left corner of the bars correctly. Right now I've got Point{height / width, height} as the top-left corner point calculation for the individual bars, but I don't know if it'll work correctly.

As for reserving the space, couldn't users also do that through the public access function I have that returns m_values?

I could just add code in the constructor that checks if the value of N is greater than the size of the vector<double>, and throw a runtime_error exception if so (I have a good function, error() that does this). Either that or I could use vector::at() to access into the vector<double> instead of vector::operator[].

[jlb]

IMO, the vector<double> should be external of your graph class and be filled in by whatever means is necessary, from a file, from standard input, or any other input method of your choice. You would then wait until after you have obtained this data to declare your graphing class using the the vector<double> as one of the parameters of your constructor.

In other words the size of the vector<double> would control the number of bars in your graph and the user would not need to worry about "counting" the number of bars required, it would be handled by the data passed to the class at the time of declaration.

By the way, other than the fact that the vector.at() throws an exception if you try to access the vector out of bounds there is no other difference between .at() and using the [] to access your vector.

[dhayden]

I also need some help on getting the top-left corner of the bars correctly.

Grab a pencil and paper and work out the formula for where the top-left corner of the i^th rectangle should go. Try a few examples by hand to convince yourself you have it right. Only then should you translate the formula to code.

As for reserving the space, couldn't users also do that through the public access function I have that returns m_values?

No because it returns a const vector.

[DragonOsman]

@jlb: By the vector<double> being external of the class, do you mean to make it a "helper" data object (variable equivalent of a helper function, I guess)? The exercise says to make the vector<double> a data member, though, which I assume means it should be a private member of the class. Am I wrong?

And yeah, I already knew that about vector.at() and the [] operator. I've tried using them interchangeably before, to try to experiment.

@dhayden: I'll try to see what I can do about the top-left corner. But I'm not good at this sort of math, so that's why I need help.

And I do still also need to know how to use the bar_spaces variable.

Edit: Updated code:

// Osman Zakir
// 7 / 4 / 2017
// Bjarne Stroustrup: Programming: Principles and Practice Using C++ 2nd Edition
// Chapter 15 Exercise 6
// chapter15ex6.cpp
// Exercise Specifications:
/**
* Design and implement a bar graph class. Its basic data is a vector<double>
* holding N values, and each value should be represented by a “bar” that is
* a rectangle where the height represents the value.
*/

#include <vector>
#include <iostream>
#include "../../Graph.h"
#include "../../Window.h"

namespace Graph_lib
{
	struct Bar_graph : Shape
	{
		Bar_graph(const std::vector<double> &values);
		void draw_lines() const;
		const std::vector<double> &values_v() const { return m_values; }
		const Vector_ref<Rectangle> &bars_v() const { return m_bars_v; }
	private:
		std::vector<double> m_values;
		Vector_ref<Rectangle> m_bars_v;
	};
}

int main()
{

}

Graph_lib::Bar_graph::Bar_graph::Bar_graph(const std::vector<double> &values)
	:m_values{ values }
{
	if (values.size() <= 0)
	{
		error("invalid number of values!");
	}
	constexpr int width = 100;
	double bar_spaces = width * 1.0 / (2 * values.size() - 1);
	for (std::size_t i = 0; i < values.size(); ++i)
	{
		int height = std::round(values[i]);
		m_bars_v.push_back(new Rectangle{ Point{height / width, height}, width, height });
	}
}

void Graph_lib::Bar_graph::draw_lines() const
{
	for (std::size_t i = 0; i < m_bars_v.size(); ++i)
	{
		m_bars_v[i].draw();
	}
}

I'm thinking of this pseudocode for the spaces:

if current rectangle and/or bar is the 0th bar/rectangle or greater and is not the last one
    add value of bar_spaces to x-value of top_left Point of the next rectangle;

But how do I make the necessary if-condition for that? What do I put there? Also, would this work?

Edit2: Updated it again. I decided to not use an if-condition since it'd be doing that anyway. I just need to get top-left corner points of the "bars" right.

Anyway, here's the code (Note: I added the bars' width as an argument to the constructor and also made the width itself a private data member. Now the user can provide a width for the bars through the constructor while also being able to do so through the setter function provided by the class):

// Osman Zakir
// 7 / 4 / 2017
// Bjarne Stroustrup: Programming: Principles and Practice Using C++ 2nd Edition
// Chapter 15 Exercise 6
// chapter15ex6.cpp
// Exercise Specifications:
/**
* Design and implement a bar graph class. Its basic data is a vector<double>
* holding N values, and each value should be represented by a “bar” that is
* a rectangle where the height represents the value.
*/

#include <vector>
#include <iostream>
#include "../../Graph.h"
#include "../../Window.h"

namespace Graph_lib
{
	struct Bar_graph : Shape
	{
		Bar_graph(const int bar_width, const std::vector<double> &values);
		void draw_lines() const;
		int bar_width() const { return m_bar_width; }
		const std::vector<double> &values_v() const { return m_values; }
		const Vector_ref<Rectangle> &bars_v() const { return m_bars_v; }
		void set_bar_width(const int value) { m_bar_width = value; }
	private:
		int m_bar_width;	// width of "bars"
		std::vector<double> m_values;
		Vector_ref<Rectangle> m_bars_v;
	};
}

int main()
{

}

Graph_lib::Bar_graph::Bar_graph(const int bar_width, const std::vector<double> &values)
	:m_bar_width{ bar_width }, m_values { values }
{
	const std::size_t N = values.size();
	if (N == 0)
	{
		error("invalid number of values!");
	}
	double bar_spaces = bar_width * 1.0 / (2 * N - 1);
	for (std::size_t i = 0; i < N; ++i)
	{
		int bar_height = std::round(values[i]);
		m_bars_v.push_back(new Rectangle{ 
			Point{ (bar_height / bar_width) + bar_spaces, bar_height }, bar_width, bar_height 
		});
	}
}

void Graph_lib::Bar_graph::draw_lines() const
{
	for (std::size_t i = 0; i < m_bars_v.size(); ++i)
	{
		m_bars_v[i].draw();
	}
}

[dhayden]

I wouldn't make the width of the bar an argument. Why should the caller have to compute the width? Why not just compute the width internally so that the bars fill up the graph?

Regarding the math, try this for a start.

Let's start with some variables:
I'll call graph.width and graph.height the width and height of the entire bar graph.
I'll call bar.width and bar.height the width & height of an individual bar.

If there are N items in the vector then bar.width = graph.width / N.

Let's let maxValue be the maximum value in the vector. If we want that value's bar to be the full height of the graph, then it should be scaled by graph.height / maxValue. In other words, to get that bar's height, you multiply maxValue * (graph.height / maxValue). The result is obviously graph.height.

To make all the other points scale the same, that means you need to multiply their values by graph.height / maxValue to get their height.

This should give us enough to figure out where each rectangle goes. In the computations below, I assume that the coordinates are all relative to the Bar_graph position. I also assume that X coordinates increase to the right and Y coordinates increase down.

The width of each bar is graph.width / N.
The left edge of the i^th bar begins at i*bar.width
The height of the ith bar is m_values[i] * graph.height / maxValue
The top of each bar is at graph.height - (the height of the bar)

Hope this helps,
Dave

[DragonOsman]

If the left edge of the i^th bar begins at i*bar.width, does that mean that the x value of the top-left corner point is (i*bar.width) + bar.height?

But about the height and width of the entire graph, I can't really find that that easily since the axes aren't included with the bar graph class and there are exercises after this one that require the use of a bar graph as well as the uses of axes and labels on the graph. I'll have to make the overall graph the right size to fit on the screen with the axes added. I'm actually thinking of just making the bars themselves, without worrying about the measurements of the entire graph.

[dhayden]

I'm assuming that the bars run vertically. If that's true then the left edge of the bar is independent of the height.

As for the height and width of the graph. The real point here is that you need to know the dimensions and position of the area where you're going to draw the graph.

[DragonOsman]

If the bars stand up vertically, wouldn't the length of the left edge, vertical, equal the height?

About the height and width of the graph. So you mean I imagine a rectangle and set the bars inside it, but do so without actually making the said rectangle on the window (I could make invisible and transparent if I wanted, so it'd be there but we couldn't see it. But in this case I'm saying there won't be such a rectangle at all)?

[dhayden]

If the bars stand up vertically, wouldn't the length of the left edge, vertical, equal the height?

The length of the left edge is equal to the height, but the x coordinate of the left edge is the same, regardless of the height. Look at the left edge your browser's window. The X coordinate of the top of the edge is the same as the X coordinate of the bottom and every point in between.

[DragonOsman]

Yeah, you're right, my bad there. So doesn't that mean that the value you gave me is the x-coordinate of the top-left point?

What about my question about the imaginary rectangle?

[dhayden]

So doesn't that mean that the value you gave me is the x-coordinate of the top-left point?

Yes, it's the x-coordinate of the top-left point, along with every other point on the left edge.

The imaginary rectangle is a fine idea.

[DragonOsman]

Now, I need a formula for the bar width that takes into account the value of the space between the bars (which will also be the space behind the first bar and the space after the last bar). Maybe graph.width / N - bar.space (bar.space is the space between the bars, as well as the space behind the first bar and the space behind the last bar), or something like that?

And this is just to make sure, but the top-left point of a bar would be given by (i*bar.width, bar.height), right? But the scaling factors with graph.width and graph.height will make the bars slightly longer, won't they?

[Note: The variable bar_spaces is defined like this: double bar_spaces = width_width * 1.0 / (2 * N - 1);, and N like this: const std::size_t N = values.size();. They're defined within the scope of the constructor.]

Edit: Here's what the constructor looks like right now:

Graph_lib::Bar_graph::Bar_graph(const int graph_width, const std::vector<double> &values)
	:m_graph_width{ graph_width }, m_values{ values }
{
	const std::size_t N = values.size();
	if (N == 0)
	{
		error("invalid number of values!");
	}
	int bar_width = graph_width / N;
	double bar_spaces = bar_width * 1.0 / (2 * N - 1);
	for (std::size_t i = 0; i < N; ++i)
	{
		int bar_height = std::round(values[i]);
		m_bars_v.push_back(new Rectangle{ 
			Point{ (i * bar_width) + bar_spaces, bar_height }, bar_width, bar_height 
		});
	}
}

The user will give the width of the graph as a constructor argument, and the class will compute the width of the each individual bar from that. How's that?

[DragonOsman]

By the way, in the case of the calculation m_values[i] * graph.height / maxValue, would be good if it were put in the constructor as the value assigned to bar_height after multiplying it by std::round(values[i]) (as in, int bar_height = std::round(values[i]) * (m_values[i] * graph.height / maxValue))? Or should I use that calculation in some other way to get the actual height of each bar?