Coordinates from array won't show up on a graph

[Miles Lacey]

Hello everyone, I'm new here but have been stuck on a problem for two days now. I wrote a small program to make an array, each element has 2 coordinates, and plot each element from that array on a graph. The only problem is, the points don't show up. Can someone please help me out?

//this program makes a graph and prints out a set of X,Y coordinates using arrays

#include <iostream>
using namespace std;
#include <iomanip>
#include <cstdlib>

struct point {
	int xCoord;
	int yCoord;
};

void calcNumbers(point* object, int);
void drawTable(point object[], int);
void drawGraph(point object[]);

int main() {
	srand(time(NULL));
	int n = 0;
	
	cout << "How many objects would you like to plot on the graph?" << endl;
	cin >> n;
	
	point* object = new point[n];

	calcNumbers(object, n); //assign 'object' random X,Y values
	drawTable(object, n);
	drawGraph(object);
	
	delete [] object;
	
	return 0;
}

void calcNumbers(point* object, int n) {
	for (int i = 0; i < n; i++) {
		//set the X,Y coordinates to a random number between -10 and +10
		// N % 21 = any number between 0 & 20
		object[i].xCoord = rand() % 21 - 10;
		object[i].yCoord = rand() % 21 - 10;
	}
	return;
}

void drawTable(point object[], int n) {
	cout << fixed << setw(3) << endl;
	cout << "  x \t | \t y" << endl;
	cout << "********************" << endl;
	for (int i = 0; i < n; i++) {
		cout << fixed << setw(3);
		cout << object[i].xCoord << "\t | \t" << object[i].yCoord << endl;
	}
	cout << endl;
	
	return;
}

void drawGraph(point object[]) {
	cout << fixed << setw(3);
	
	//make a 10x10 grid to plot points onto
	for (int y = 10; y > -11; y-=1) { //y-values go from positive to negative
		for (int x = -10, i = 0; x < 11; x+=1, i++) { //x-values go grom negative to positive
			//draw a point based on the x,y values
			if (x == object[i].xCoord && y == object[i].yCoord) {
				cout << " o ";
			}
			//draw the origin
			else if (x == 0 && y == 0) {
				cout << "-|-";
			}
			//draw y-axis where x=0
			else if (x == 0) {
				cout << "-|-";
			}
			//draw x-axis where y=0
			else if (y == 0) {
				cout << "-|-";
			}
			//assign a blank space to any empty points on the graph
			else {
				cout << "   ";
			}
		}
		cout << endl;
	}
	return;
}

I have also rewritten the program without using arrays and it graphs the points just fine. Not really sure what the problem is though. Thanks in advance to anyone who can help.

[andywestken]

I'd look at these lines (64-, as orig posted)

			//draw a point based on the x,y values
			if (x == object[i].xCoord && y == object[i].yCoord) {
				cout << " o ";
			}

I'm not convinced you can handled the x and i loop variables using the same for(...)

Andy

[Miles Lacey]

Thanks Andy.
I tried eliminating the i loop variable, but there program still doesn't plot anything.

		for (int x = -10; x < 11; x+=1) { //x-values go grom negative to positive
			//draw a point based on the x,y values
			if (x == object[x].xCoord && y == object[y].yCoord) {
				cout << " o ";
			}
			...

[andywestken]

I didn't mention anything about eliminating i.

I think you need a new, separate loop for the i variable, which searchs the array of random points for a match with the current coord.

This is because your calcNumbers() function allocates points randomly, so the xCoord for object[0] is highly unlikely to correspond to the first point (it has an equal chance of being any value between -10 and 10 and hence only an approx a 5% chance of being -10)

You updated code is worse than it was. Now you are accessing arrays elements using a -ve number, when the first array elem is always 0.

I suggest you:
a. add logging to the drawGraph, telling you what it's doing
b. revise C++ array handling

[Miles Lacey]

It works! I did what you said and added another for loop to search through the array lists. I also made some other small changes, nothing major. Here's the final portion of the function:

void drawGraph(point object[], int n) {
	cout << fixed << setw(3);
	bool bGraphed = 0;
	string output = "   ";
	
	//make a 10x10 grid to plot points onto
	//y-values go from positive to negative
	for (int y = 10; y > -11; y-=1) {
		//x-values go grom negative to positive
		for (int x = -10; x < 11; x+=1) {
			//determine if x,y coordinate must be drawn
			for (int i = 0; i < n; i++) {
				if (x == object[i].xCoord && y == object[i].yCoord) {
					bGraphed = 1;
				}
			}
			//draw the origin
			if (!x && !y && !bGraphed) {
				output = "-|-";
			}
			//draw y-axis where x=0
			else if (!x && !bGraphed) {
				output = "-|-";
			}
			//draw x-axis where y=0
			else if (!y && !bGraphed) {
				output = "-|-";
			}
			//print a point if there is an object
			else if (bGraphed) {
				output = " o ";
			}
			//assign a blank space to any empty points on the graph
			else {
				output = "   ";
			}
			//print an output 
			cout << output;
			bGraphed = 0;
		}
		cout << endl;
	}
	return;
}

[andywestken]

Your final code is nice and clean (as in, easy to understand) but there are a few improvements you could make. 1 - 7 are the keys ones. The other three prob more a matter of taste.

1. you should use clearer names for your function params and some of your variables.

(And I like to see param names in forward definitions, so I get a better idea of what data is being passed to the function, without having to find the defnition)

2 bool variables should be set to either true or false

3.as you're outputting a fixed set of const string, I would prob not use std::string here

Each time you call string::operator= you're copying a string from its char buffer to heap memory. If you were building a longer string, this would make sense, but here I would make "ouput" a const char*

4. setw only applies to the first insertions after it. so this line

cout << fixed << setw(3);

should lose the - pointless - setw()

Same with first line in drawTable()

See: http://www.cplusplus.com/reference/iostream/manipulators/setw/

But fixed applies until it is changed to scientific, so you don't need to use it before each insertion, unless you use a mixture of fixed and scientific

5. It would prob be better to use constants for the graph range.

(Or even variables)

6. I would prob test bGraphed separately to your other condition, so you don't repeat the same test so often.

(I also prefer to test independent decisions separately: here you asking (a) am I plotting a point or not and (b) if not, am I on an axis)

7. I would use const for the first param of your two drawing functions

void drawTable(const point object[], int);
void drawGraph(const point object[]);

8. For consistency, I would define calcNumbers as:

void calcNumbers(point object[], int);

(or use point* for all functions)

9. The return at the end of the functions returning void could be removed. You only need a return; if you want to exit the function early.

(It's benign but not normally done. As a lot of people are lazy?)

10. Finally, a matter of personal taste. I would use "x == 0" where you use !x. This is because it makes it clearer to me what's happening (I only ever use !x style for bool values)


Andy

[andywestken]

And you could tweak the loop which searches for points so it bails out as soon as it know there's a match (the && !bMatched)

			//determine if x,y coordinate must be drawn
			for (int i = 0; (i < n) && !bGraphed; i++) {
				if (x == object[i].xCoord && y == object[i].yCoord) {
					bGraphed = true;
				}
			}