# [C++ std::vector] deleting a specific part from the list [SOLVED]

This topic is 3583 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

## Recommended Posts

As elementary as this may seem, I'm having a hard time deleting a part from a vector list. I was writing this "shop" buying text program for fun and came across this problem. Below is my code:
// Buying system 1.0

#include <iostream>
#include <string>
#include <vector>

using namespace std;

int hero_capacity = 2;
vector<string> heroInventory;
vector<string>::const_iterator hInventoryCounter;
vector<string> inStock;
vector<string>::const_iterator stockCounter;
string input;

void create_shop_list(vector<string>& );
void buy_from_shop(string , vector<string>&, vector<string>& );
void print_shop_list(const vector<string> );
void print_hero_list(const vector<string> );
const char* make_char_from_string(const string& );

void say_goodbye( void );

int main()
{
cout << "generating shop stock...\n\n";
create_shop_list(inStock);

do
{
cout << "This is your inventory:\n";
cout << "-----------------------\n\n";
print_hero_list(heroInventory);
cout << "You can only hold " << hero_capacity << " items!\n\n";

cout << "\n\nWelcome to me shop! Choose what gets your eye or type in \"quit\" to quit:\n";
print_shop_list(inStock);
cout << ">_ "; //To show where you type
cin >> input;
}while( true ); //forever loop

return 0;
}

void create_shop_list(vector<string>& list)
{
list.reserve(5);
list.push_back("\n");
list.push_back("sword");
list.push_back("hammer");
list.push_back("backpack");
list.push_back("armor");
list.push_back("tank");
}

void buy_from_shop(string item, vector<string>& list, vector<string>& hero_list)
{
if( item.compare("quit") == 0 ) //not an item, but a command
{
say_goodbye();
}

if( list.empty() )
{
cout << "You bought all of my things. Sorry.\n\n";
return; //Do not continue the rest of this function
}

bool found = false;
vector<string>::iterator counter;

for( counter = list.begin(); counter < list.end(); counter++ )
{
if( (*counter).compare(item) == 0 ) //chosen item was in shop
{
if( (*counter).compare("backpack") == 0 )//special item
{
found = true; //aparently, the item was found
list.erase( counter );
hero_capacity += 2; //increase how much you can hold
cout << "With that, you can carry more things!\n\n";
}
else
{
if( hero_list.size() <= (unsigned)hero_capacity )
{
found = true; //aparently, the item was found
list.erase( counter );
hero_list.push_back( (*counter) );
}
else
{
found = true; //We found it, but we can't get it. :/
cout << "\nIt looks like you're full. Sorry. \n\n";
}
}
}
}

if(!found) //if the item wasn't found at all
{
cout << "\nSorry, we don't sell that here.\n\n";
}
}

void print_shop_list( const vector<string> list )
{
if( list.empty() )
{
cout << "(none)\n\n";
return;
}

for( stockCounter = list.begin(); stockCounter < list.end(); stockCounter++ )
cout << make_char_from_string( (*stockCounter) ) << "\n";
}

void print_hero_list( const vector<string> hList )
{
if( hList.empty() )
{
cout << "(none)\n\n";
return;
}

for( hInventoryCounter = hList.begin(); hInventoryCounter < hList.end(); hInventoryCounter++ )
cout << make_char_from_string( (*hInventoryCounter) ) << "\n";
}

const char* make_char_from_string(const string& _string)
{
int total_size = 0;
string::const_iterator _string_it;

for(_string_it = _string.begin(); _string_it < _string.end(); _string_it++ )
total_size += 1;

char* r_string = new char[total_size];

for(int j = 0; j < total_size; j++ )
{
r_string[j] = _string[j];
}

r_string[total_size] = '\0';

return r_string;
}

void say_goodbye( void )
{
cout << "Well, thanks for shoppin'!\n\n";
exit(0);
}


The problem is the buy_from_shop() function. I loop through the list looking for the entered object and, if found, delete it from the shop's inventory and then put it into the hero's stash. So if any help could be given, that would be great. Also, I'm aware of the other problems, so you don't have to inform me of those ( code wise ) ~Maverick [Edited by - PCN on August 3, 2008 10:58:40 AM]

##### Share on other sites
The problem is that your call to erase on the shop's stock vector invalidates the iterator pointing to that element, and then when you try to add it to the hero's inventory it's not actually pointing to anything because you just deleted it.

The simplest fix, since you're just using strings, would be to copy the item to the hero's inventory and *then* delete it from the shop.

If the inventory system is to be more sophistcated you could implement a wider-scoped collection of objects in the 'world' and then transfer ownership of that object from the shop to the hero.

##### Share on other sites
There is no need to write complicated things like
if( (*counter).compare(item) == 0 )
, when the string class is specifically designed to make perfectly natural things like
if (*counter == item)
work exactly as you would expect them to. The .compare member function is provided for special circumstances. You might easily never need it.

Similarly, you don't need to write a function in order to convert a string to a const char* (you can use .c_str() if the rare occasion ever comes up that you need this; and if you get really unlucky and need a mutable copy, there are still much simpler ways to write the function), and you don't need to convert a string to a char* to output it anyway (you just feed the string directly to operator<<).

Also, don't use global variables for iterators. It allows you the chance to write all kinds of really bizarre bugs, without ever actually being useful.

But anyway, the standard library provides not only data structures, but algorithms which can greatly simplify your tasks.

#include <iostream>#include <string>#include <vector>#include <algorithm>#include <iterator>using namespace std;int hero_capacity = 2;// As it stands, nothing else needs to be global, and we could even fix this// one, too; but for now it's easiest to leave it alone.// But in general, making something global should be your *last* thought of// what to do with a variable.// There is no good reason to put a blank item into the list. The .reserve()// call is not necessary if you're going to insert all your items with// .push_back(); it's only a performance optimization, which is mind-bogglingly// useless here. And even then, reserving space for 5 elements is no help when// you were going to push_back 6 of them. Moral: don't prematurely optimize.void create_shop_list(vector<string>& list) {	list.push_back("sword");	list.push_back("hammer");	list.push_back("backpack");	list.push_back("armor");	list.push_back("tank");}// Use return values from functions to communicate information. Especially// important things like "we are done". Exiting from an infinite loop with// an exit() call, indirectly a few functions deep, as the normal way of// ending the program, is incredibly bad practice.bool buy_from_shop(const string& item, vector<string>& list, vector<string>& hero_list) {	if (item == "quit") { //not an item, but a command		cout << "Well, thanks for shoppin'!\n\n";		return false;	}	if (list.empty()) {		cout << "You bought all of my things. Sorry.\n\n";		return true;	}	// The important thing here is whether the thing was found: figure that	// out first, instead of nesting code deeply. Here, a standard library	// algorithm helps out, avoiding the need to write a loop.	vector<string>::iterator selection = std::find(list.begin(), list.end(), item);	if (selection == list.end()) { // not found		cout << "\nSorry, we don't sell that here.\n\n";		return true;	}	if (*counter == "backpack") { //special item		hero_capacity += 2; //increase how much you can hold		cout << "With that, you can carry more things!\n\n";		list.erase(selection);	} else if (hero_list.size() > (unsigned)hero_capacity) {		cout << "\nIt looks like you're full. Sorry. \n\n";	} else {		// Notice how the bug is addressed here: we copy the item before		// destroying it rather than after.		hero_list.push_back(*selection);		list.erase(selection);	}	return true;}// There's no need for a separate function to print the shop list and the hero// list, because they are printed in exactly the same way. This is why we make// functions accept parameters: so that we can do the same things with varying// sets of data.void display_list(const vector<string>& list) {	if (list.empty()) {		cout << "(none)\n\n";	} else {		// Thanks to standard library algorithms, we don't need to		// write a loop to output vectors either:		copy(list.begin(), list.end(), ostream_iterator<string>(cout, "\n");	}}int main() {	cout << "generating shop stock...\n\n";	vector<string> heroInventory, inStock;	create_shop_list(inStock);	string input;	do {		cout << "This is your inventory:\n";		cout << "-----------------------\n\n";		display_list(heroInventory);		cout << "You can only hold " << hero_capacity << " items!\n\n";		cout << "\n\nWelcome to me shop! Choose what gets your eye or type in \"quit\" to quit:\n";		display_list(inStock);		cout << ">_ "; //To show where you type		getline(cin, input); // use this to get a whole line of text		// instead of just a word... trust me, in the long run you'll		// be much better off always reading cin a line at a time	} while buy_from_shop(input, inStock, heroInventory);	// Notice how the return value from the function is used to control the	// loop: this makes it perfectly obvious what is going on.}

##### Share on other sites
Wow, Zahlman. I feel like an idiot now. We have our days. :]
Thank you for the help, though! I didn't know there was a .c_str() function. That would have saved me in the long run. And thanks for the algorithm! I know that'll help me a ton in my real projects!

~Maverick

• 10
• 16
• 9
• 13
• 41