Maximal Coordinate searching

[Auroch]

Hello everyone! I'm trying to write program with vectors and iterators. Coordinates X and Y are entered from keyboard (as structure) and placed in vector of points. It's necessary to display coordinates with maximal X and Y. This code was compiled but not display maximal X. What should I change?

#include <iostream.h>
#include <conio.h>
#include <vector.h>
#include <math.h>

struct points {
	float x, y;
};

int main()
{
using namespace std;

int i;
int max;

vector<points>pointvect; //vector declaration
points point;

cout << "Enter a dimension of vectors:\n";
cin >> i;

for (int k = 0; k <= i; k++) {
	cout << "Inpurt x\n";
	cin >> point.x;
	cout << "Input y\n";
	cin >> point.y;
}

pointvect.push_back(point); // add point to the vector


// define iterators

vector<points>::iterator itr;
vector<points>::iterator first = pointvect.begin();
vector<points>::iterator end = pointvect.end();

(first)->x = max;

for (itr = first; itr <= end; itr++) {
        if ((itr)->x > max) {
		cout << "Maximal X is: ", (itr)->x;
	}
}
getch();
return 0;
}

[MiiNiPaa]

Your max value is not initializated. that means, it contain garbage value.
Furthermore your code do not change it even if it finds something larger.

[tipaye]

There are several problems:

in lines 23-30, you are only ever adding the last point to the vector.

in line 39, you are modifying the point, setting it to an undefined value, I think you meant to swap the positions of the operands.

in line 43, the logic doesn't add up, the first thing that's greater than (still uninitialised) max is called the greatest. You should instead, check everything, replacing max with anything larger that you find. That way, at the end, you'll find that max is the biggest.

Still on line 43, what you have is essentially 2 statements, you have not actually tried to output the value.

You will likely never execute line 43 in any case, if the random value in max is greater than anything entered.

in line 23, by starting from "0" and using "<=", you will actually create 1 extra element in the vector than the user intended, So if they entered a dimension of 5, they end up with 6 points instead.

in line 41, this not how to compare iterators, by "using <=", you end up de-refrencing "end()", which is not a valid part of your container. "end()" points to one location past the end of your container.

Minor annoyances which may just be a question of personal taste include the placement of your using directive, dubious data types (why is max an int?) gratuitous declaration of iterators and surrounding them in parentheses, e.g. (itr)->x, and a style that suggests your C++ implementation is several centuries old.

Have a look at the bare-bones modifications below:

#include <iostream.h>
#include <conio.h>
#include <vector.h>
#include <math.h>

using namespace std;			

struct points {
	float x, y;
};

int main()
{
	cout << "Enter a dimension of vectors:\n";
	int i;
	cin >> i;

	vector<points>pointvect; //vector declaration
	points point;
	for (int k = 0; k < i; k++) {	
		cout << "Input x\n";		
		cin >> point.x;
		cout << "Input y\n";
		cin >> point.y;
		pointvect.push_back(point); // add point to the vector
	}

	float max = 0.0;

	for (vector<points>::const_iterator itr = pointvect.begin(); itr != pointvect.end(); ++itr){
		if (itr->x > max) {
			max = itr->x;
		}
	}
	
	cout << "Maximal X is: " << max << endl;
	getch();
	return 0;
}

[Auroch]

How about this? Is it better?

(first)->x = max;
for (itr = first; itr <= end; itr++) {
        if ((itr)->x > max) {
                if ((itr)->x = max;
		cout << "Maximal X is: ", (itr)->x;
	}

Your max value is not initializated. that means, it contain garbage value.

It's likely that I initialize max at the line 15 int max;.
Or it's not that?

[Chervil]

It's likely that I initialize max at the line 15 int max;.

if you do this:

    int max;
    cout << max;

what value will be displayed?

[Auroch]

@Chervil.
value

4219551

What is it?

[Auroch]

Similar to maximal int: INT_MAX Maximum value for an object of type int 32767 (215-1) or greater*

[Chervil]

Because no initial value was assigned to max, it will contain whatever garbage value happened to be already in that memory location. It could be anything at all, and might change each time the program is recompiled.

Instead, a specific value should be given. For example,

int max = 0;

here the variable is properly initialised. Of course we might want a different value. It's up to the programmer to decide.

[Auroch]

I've declared (for test)

max = 0;

for (itr = first; itr <= end; itr++) {
        if ((itr)->x > max) {
                if ((itr)->x = max;
		cout << "Maximal X is: ", (itr)->x;
	}

And the output is without value:

Maximal X is:

[Chervil]

That code has errors:
if ((itr)->x = max;
where is the matching right bracket ')' ?
did you intend == rather than = ?

Here, the comma operator may not be what you meant:

    cout << "Maximal X is: ", (itr)->x;

perhaps instead

    cout << "Maximal X is: " <<  (itr)->x;

[Auroch]

Thanks for the help, guys! You are the best!

[keskiverto]

Now that you have solved it, try to guess what this does:

auto maxbyx = std::max_element( pointvect.begin(), pointvect.end(),
                               []( const & point lhs, const & point rhs ) { return lhs.x < rhs.x; } );
std::cout << "Point with maximal X is (" << maxby->x << ", " << maxbyx->y << ")\n";