C++ using vector objects.

Started by
4 comments, last by DraconisRavenix 16 years, 6 months ago
Well, I am a complete n00b to C++. I am currently taking an OOP with C++ class and I ran into a problem with using vectors. Any help would be greatly appreciated.

// standard headers
#include <iostream>		// include input-output stream
#include <conio.h>		// control return with user keystroke using getch();
#include <iomanip>		// allow manipulation (formatting) for output
#include <string>		// include string header
#include <fstream>		// include file stream header
#include <vector>		// include header file to use the vector class (in STL)

// standard namespace
using namespace std;

// function prototypes
void getData(vector<int>& data, int num);
void remove(vector<int>& data,int num,int delThis);


// main() function
int main()
{
	// variables
	int num = 10;
	int delThis;
	int counter;
	vector<int> data(num);
	vector<int>::iterator vecIter;

	getData(data,num);

	cout << "data that was input:" << endl << endl;

	for (counter = 0; counter < num; counter++)
	{
		cout << data[num] << endl;
	}

	cout << endl << endl << endl;
	cout << "Please enter a number to remove from the list: ";
	cin >> delThis;
	cout << endl << endl << endl;

	remove(data,num,delThis);

	cout << endl << endl;
	cout << "After searching and erasing your number " << delThis << ", here is the list of data:" << endl;
	for (counter = 0; counter < num; counter++)
	{
		cout << data[num] << endl;
	}



	// Return to system
	cout << "Press any key to exit.";
	_getch();
	return 0;
}


// secondary functions
void getData(vector<int>& data,int num)
{
	int counter;
	ifstream infile;

	infile.open("data.txt");

	for (counter = 0; counter < num; counter++)
	{
		infile >> data[counter];
	}
}

void remove(vector<int>& data,int& num,int delThis)
{
	int counter;
	int swapThis;
	bool found = false;

	for (counter = 0; counter < num; counter++)
	{
		if (data[counter] == delThis)
			found = true;
	}

	if (found == false)
		cout << "Number " << delThis << " was not found." << endl;
	else
	{
		cout << "Number " << delThis << " was found. Deleting..." << endl;
		for (counter = 0; counter < num; counter++)
		{
			if (data[counter] == delThis)
			{
				data.erase(counter);
				num = data.size();
			}
		}
	}
}
The Problem I am having is this error message at compile-time: c:\cis 247\assignment_4_3\assignment_4_3\assignment_4_3.cpp(94) : error C2664: 'std::_Vector_iterator<_Ty,_Alloc> std::vector<_Ty>::erase(std::_Vector_const_iterator<_Ty,_Alloc>)' : cannot convert parameter 1 from 'int' to 'std::_Vector_const_iterator<_Ty,_Alloc>' with [ _Ty=int, _Alloc=std::allocator<int> ] No constructor could take the source type, or constructor overload resolution was ambiguous I apologize if it is a bit hard to read. I am a little slow and lacking in the department of organization and documentation. At the end of the code within th function 'void remove()' in the 'if' statement I want to use the 'erase()' function within the vector object of 'data'. Again any and all help (including constructive criticism) will help. Thanks in advance.
Knowledge is power. Power corrupts. Study hard.
Advertisement
erase() takes an iterator, not an index. You should change the line
data.erase(counter);
to:
data.erase(data.begin() + counter);
Also, your for loop is a little dodgey. If you erase an element at index 3, then the next element is now at index 3, but your loop increments counter to 4. So you don't do any processing on the element after the one deleted. It's less of an issue of you know that delThis is only going to exist once in the vector, but it's still good practice to change it:
for (counter = 0; counter < num;){   if (data[counter] == delThis)   {      data.erase(counter);      num = data.size();   }   else      ++counter;}


And, as another minor nit-pick, it's a good idea to use iterators instead of indices. That'll let you easily change container (E.g. to std::list) if you want to later on. The revised code would be:
bool remove(vector<int>& data, int delThis){   bool found = false;   for(vector<int>::iterator it=data.begin(); it!=data.end();)   {      if(*it == delThis)      {         // erase returns an iterator to the next element         it = data.erase(it);         found = true;      }      else      {         // ++it can sometimes be slightly more efficient than it++, because         // a temp. object doesn't need to be made         ++it;      }   }   if(found)      cout << "Number " << delThis << " was found. Deleting..." << endl;   else      cout << "Number " << delThis << " was not found." << endl;   return found;}

Untested, might have a but or two in it.

EDIT: Changed the function slightly - there's no point in filling in the size of the vector, the function that called remove() can just check the size of the vector itself. I also changed it to return true if the element was found and removed, else false. Maybe not useful, but you might as well return it in case it's needed.

I'm sure someone else will come along with a std::remove_if() solution [smile]
As the compiler and documentation mention, erase accepts an iterator. Also, your code does not take into account element removal in its iteration.

You should consider using standard library algorithms for something so simple:

void remove(vector<int>& data,int& num,int delThis){  cout << "Number " << delThis <<     (std::find(data.begin(),data.end(),delThis) == data.end()      ? " was not found." : " was found. Deleting...") << endl;  data.erase(    std::remove(data.begin(),data.end(),delThis),    data.end());  num = static_cast<int>(data.size());}
I didn't go through all your code, just where the problem is. std::vector::erase takes iterators to the position where you want to delete (and returns an iterator to the next element). So bascially to make this compile change the line:

data.erase(counter);


to:

data.erase(data.begin() + counter);


std::vector::begin returns an iterator to the first element, std::vector's iterator are random accessible so you can offset them in constant time like you can with pointers.

A little advice, some of your functions already exist in standard library form, either as member functions of std::vector or as standard library generic algorithms (like std::remove/remove_if) which will work with any standard library container and even plain C-style arrays/strings.

Try finding a good reference/book like this.
Wow....too much to take in all at once!! I love it!

Thank you all for the help, I'm going to go back through my code as well as look more into the STL documentation, such as the http://www.sgi.com/tech/stl/table_of_contents.html from snk_kid.

I'm just surprised as heck to find this many responses so quick!

Again, thanks everyone for the help! I'll post my results and what I've learned later when I have a bit more time.
Knowledge is power. Power corrupts. Study hard.
OK, it's been a while but, I did say I'd respond with what I've learned and implemented. First though, I have a question regarding Evil Steve's comment within the code:

Quote://++it can sometimes be slightly more efficient than it++, because
// a temp. object doesn't need to be made


Like I said, complete n00b taking a C++ course. In our class we didn't really cover the difference between
var++
and
++var
. What exactly is the difference that makes an impact on this code specifically? Can anyone elaborate on the comment above? I know that in expressions (manipulating multiple variables) it makes a difference but, to just increase the value of something by one (in a stand-alone form) does it really matter?

I know, a lot of questions but, I don't like doing stuff just to say I did it and it works; I have to learn something or it's useless.

Oh, and by the way ToohrVyk and snk_kid, I like the examples/tips but, I haven't gone too much into the STL portions of C++. I'm keeping with the basics for now and still getting a feel for the coding, syntax, logic etc of C++. I do understand what your code is doing but, I'm a little fuzzy as to how and why. snk_kid, I did bookmark that link for future references. I'm just attempting to learn the language in steps, everything else that I've learned in leaps seems to have enough missing gaps to keep that knowledge from being fully applicable.


EDIT: I knew I saw something that kind of had to do something with STL and n00bs like me, finally found it again here.

After seeing this I thought I'd add my own little comment: I am at one of the most n00bish of levels in C++. Syntax and coding in this language (as well as any other language) takes practice. Going the long way around (STL gives a LOT of shortcuts in certain ways) and typing every little function out manually and/or recreating my own objects, functions, etc which are already in the STL gives the needed practice. This is my opinion and others obviously disagree but, to each their own! :)

[Edited by - DraconisRavenix on October 2, 2007 5:37:13 AM]
Knowledge is power. Power corrupts. Study hard.

This topic is closed to new replies.

Advertisement