Vector Iteration Problem

[Teelnaw]

I am having some issues around iterating and incrementing through a vector, both it's element and it's values. What I am trying to do in this mess below is take the first and second elements of a vector, compare them and then swap then if one value is less then the other. I am getting two main errors, one "out of range memory location" and the other was that I can't increment a vector iterator. Could anyone point me in the right direction here? Sorry for the messy code, I've been at this for a while and when trough a number of different possibilies.

#include <iostream>
#include <vector>
#include <algorithm>
using namespace std;

void swap(vector<int> *grade);
int main ()
{
	vector<int> grade, hist;
	unsigned int x;
	int i = 0;
		
	cout << "Enter a list of grades (positive numbers). \n"
		<< "Enter a negative number at the end. \n";

	int next;
	cin >> next;
	while (next > 0)
	{
		grade.push_back(next);
		cin >> next;
	}

	
	//Swap grades 
	
	swap(&grade);	

void swap(vector<int> *grade)
{
	
	int idx = 0;
	int temp;

		
		for(vector<int>::iterator i = grade->begin(); i != grade->end(); ++i)
			for(vector<int>::iterator j = grade->begin()+1;; ++j)
			{
				if(grade->at(idx) > grade->at((idx) + 1))
				{ cout << grade->at(idx) <<  grade->at((idx) + 1);}
				else
				{
					temp = grade->at(idx);
					grade->at(idx) =  grade->at((idx) + 1);
					grade->at((idx) + 1) = temp;
				}

				++count;
			}
				
	

		cout << "count " << count <<endl;
		cout << "Grade 1 " <<  grade->at(idx) << endl;
		cout << "Grade 2 " << grade->at((idx) + 1) << endl;

		system("PAUSE");


}

[Teelnaw]

Would this be easier if it were in main? I am creating more problems by passing the vector to a function?

[shacktar]

Your main function is missing a closing brace and the "count" variable in the swap function is not declared. I don't know about the "can't increment a vector iterator" error. Could you post the error exactly as it is shown?

One major problem I found is in this code:

for(vector<int>::iterator j = grade->begin()+1;; ++j)

You did not give this for loop a condition so "j" is being incremented too far.

Also I assume that you will want to do more than just check the first two elements over and over again. Remember to increment idx accordingly.

[Teelnaw]

Disregard the syntax in main. I cut it up for this post. Assuming main is correct. Can what I am trying to do in the swap function be done as it is here. Yes you are correct in saying that I want to check more then the first two elements. The idea would be to check all elements from i+1 to the end of the array against the first element (i). Then move the smallest (or largest value to the beining of the vector) and continue to do so until the elements are ordered.

The compiler doesn't like me iterating the vector iterator (++i), but I figure out why the I am getting the memory out of range. The for loop condition needs to be less one (eg. grade->end()-1).

Thanks for the input.

[shacktar]

Is the compiler only giving you a warning for saying ++i? If so, ignore it, that is the standard way to iterate over a vector. Like I said, the out of range (assuming you have at least two elements in the vector) is happening because your second for loop does not have a terminating condition.

[Teelnaw]

OK

I got passed the errors and compiler issues. When it compiles the vector isn't iterating properly. idx + 1 is adding one to the value instead of moving to the next element. How do I access the vector so that I can move element positions? Same with the if statement it's comparing element i (say 45) to i + 1 (46)

Thanks for your help!! Much appreciated.

void swap(vector<int> *grade)
{
	unsigned int x = grade->size();
	int count = 0;
	int idx = 0;
	/*int i = grade->at(idx);
	int j = grade->at(idx)+1;*/
	int temp;

	while (count < grade->size()-1)
	
		for(vector<int>::iterator i = grade->begin(); i != grade->end()-1; ++i)
			for(vector<int>::iterator j = grade->begin()+1;j != grade->end()-1; ++j)
			{
				cout << "outside if " << grade->at(idx) <<  grade->at(idx) + 1 << endl;
				
				if(grade->at(idx) > grade->at(idx) + 1)
				
				{
					
					cout << "Inside If " << grade->at(idx) <<  grade->at(idx) + 1 << endl;
				}

				else
				{
					temp = grade->at(idx);
					grade->at(idx) =  grade->at(idx) + 1;
					grade->at((idx) + 1) = temp;
				}

				
				++count;
			}
				
	

		cout << "count " << count << "\n";
		cout << "Grade 1 " <<  grade->at(idx) << "\n";
		cout << "Grade 2 " << grade->at(idx) + 1 << "\n";
		cout << "Grade 3 " << grade->at(idx) + 2 << "\n";

		system("PAUSE");


}

[shacktar]

Bracket placement is everything.

this:

grade->at(idx) + 1

says "take the element at index, idx. Then, add 1 to it"

whereas this:

grade->at(idx + 1)

says "take the element at index, idx + 1 (i.e. the element just after idx)".

In the parts of the code where you want to compare with the next element, you should use the second approach.

[Teelnaw]

How about for the following. grade->end()-1. Will this give me end - 1 element or the end value -1?

for(vector<int>::iterator i = grade->begin(); i != grade->end()-1; ++i)
for(vector<int>::iterator j = grade->begin()+1;j != grade->end()-1; ++j)

[LB]

It gives you the last element in the vector. You don't need the -1 in that case, though, because you'd never get to the last grade at all.

I have a question: why do you need to use vector iterators anyway?
Can't you just use for(vector<int>::size_type i = 0; i < MyVector.size(); ++i) and access the elements in the vector with the subscript [] operator?

[Zhuge]

You don't need to, but if you switch to another container that doesn't support subscript random access, then you won't need to rewrite the code to use iterators.

[D80DSL3A]

I think the main problem is that you aren't using the iterators (i,j) at all in the loop body!
The code you have here:

for(vector<int>::iterator j = grade->begin()+1;j != grade->end()-1; ++j)
{
	cout << "outside if " << grade->at(idx) <<  grade->at(idx) + 1 << endl;			
	if(grade->at(idx) > grade->at(idx) + 1)				
	{					
		cout << "Inside If " << grade->at(idx) <<  grade->at(idx) + 1 << endl;
	}
	else
	{
		temp = grade->at(idx);
		grade->at(idx) =  grade->at(idx) + 1;
		grade->at((idx) + 1) = temp;
	}				
	++count;
}

should be like this:

for(vector<int>::iterator j = grade->begin()+1;j != grade->end()-1; ++j)
{
        //   j IS THE ITERATOR!	
	cout << "outside if " << *j <<  *(j+1) << endl;

	if( *j > *(j+1) )				
	{					
		cout << "Inside If " << *j <<  *(j+1) << endl;
	}
	else
	{
		temp = *j;
		*j = *(j+1);
		*(j+1) = temp;
	}				
	++count;
}

[Teelnaw]

Ok so this one compiles with no issues. However it is looping to many times. For 3 inputs, it loops 6 times when on the 4 loop its correct. How do I contain the nested loops.

Thanks

void swap(vector<int> *grade)
{
	unsigned int x = grade->size();
	unsigned int count = 0;
	/*int idx = 0;*/
	/*int i = grade->at(idx);
	int j = grade->at(idx)+1;*/
	int temp;

	cout << "grade size " << x;

	while (count < x)
	{
		
	
		for(vector<int>::iterator i = grade->begin(); i != grade->end(); ++i)
			for(vector<int>::iterator j = grade->begin()+1; j != grade->end(); ++j)
			{
				cout << "outside if " << *i <<  *j << endl;
				++count;		
				if(*i > *j)
				
				{
				   cout << "Inside If " << *i <<  *j  << endl;
				   
					
				}

				else
				{
					temp = *i;
					*i =  *j;
					*j = temp;
					cout << "Else STM " << *i <<  *j << endl;
					 							
				}

				
			
			
				
			cout << "count " << count << "\n";
			cout << "grade[0] " << grade->at(0) << "\n";
			cout <<  "grade[1] " << grade->at(1) << "\n";
			cout <<  "grade[2] " << grade->at(2) << "\n";
			
			
						
			
			}
		
	}

	

		system("PAUSE");

	
}

[shacktar]

You're doing a bubble sort, right?

for(size_t lastOffset=grade->size()-1; lastOffset>= 1; lastOffset--)
{
     for(vector<int>::iterator itr = grade->begin(); itr != grade->begin()+lastOffset; ++itr)
     {
          if(*itr > *(itr+1))
          {
                //swap
          }
     }
}

[Teelnaw]

Shacktar that worked !! Thanks

So how would you then compare two elements in a vector.

(ie)
Input 45, 85, 95.
// check for equality among all input elements. See if 45 is equal 85, then 95. then check to see if 85 is equal to 95. I tried to use a nest loop as in my previous post but it does work properly.

[shacktar]

Well, you would iterate from the first to the second last element. First, set a bool (allEqual) to true. In each iteration, check if *itr == *(itr + 1). If they're not equal, set allEqual to false. You don't have to check any more because you know all the elements can't be equal if one pair is not. I won't give the code because at this point you should be able to code the loop yourself.