Sign in to follow this  

STL vector::erase (iterator) erases the last element

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

If you intended to correct an error in the post then please contact us.

Recommended Posts

Hello, Here is a part of my code:
char AARemove (char *alias) {
	vector<AALIAS>::iterator it;
	AALIAS aa;

	if (alias != NULL) {
		aa.Key = alias;
		
		it = find (AAliases.begin (), AAliases.end (), aa);
		
		printf ("Removing \"%s\" - \"%s\"\n", alias, it->Key.c_str ());

		if (it != AAliases.end ()) {
			// Remove it from the aliases list
			printf ("Done: \"%s\"\n", AAliases.erase (it)->Key.c_str ());
			
			return 2;
		}
		return 1;
	}
	return 0;
}


Where vector<AALIAS> AAliases; The AALIAS class looks like this:
class AALIAS {
	public:
		string Key;
		string Command;

		bool operator<(const AALIAS &a) const;
		bool operator==(const AALIAS &a) const;
		AALIAS operator=(const AALIAS &a) const;

		bool Set (char *cmd);
		bool Add (char *cmd);
		void Clear ();
};


Its operators:
bool AALIAS::operator<(const AALIAS &a) const {
	return (Key < a.Key);
}

bool AALIAS::operator==(const AALIAS &a) const {
	return (Key == a.Key);
}

AALIAS AALIAS::operator=(const AALIAS &a) const {
	AALIAS aa;
	aa.Key = a.Key;
	aa.Command = a.Command;
	return aa;
}


AAliases vector contains the following: "cli" "cast lightning" "ccl" "cast continual" "ccf" "cast create" "ccs" "cast create spring" "cac" "cast acid" "css" "cast stone" "cl" "cast cure light" "cb" "cast blindness" "cgs" "cast giant" "cs" "cast shield" Where the first strings are the Keys and the second are Commands. Now the problem is that when called with argument "cac", the function AARemove prints out this: Removing "cac" - "cac" Done: "cac" Which clearly indicates that the entry with Key "cac" wasn't removed. Furthermore, for some reason it removed the last element of the AAliases vector. However, when it->Key.c_str () is equal to "cac", then the iterator just can't be pointing to the last element, can it? It seems that I'm doing something wrong, as according to the documentation, vector::erase shouldn't behave that way.. All suggestions are welcome, Thank You [Edited by - IndrekSnt on May 26, 2008 8:25:21 AM]

Share this post


Link to post
Share on other sites
I see two problems:

(1) AALIAS operator=(const AALIAS &a) const, Why is your assignment operator a const member function? It modifies the object, does it not?

(2) The return value from std::vector::erase() is the element after the last element erased.

Share this post


Link to post
Share on other sites
Thank You for Your reply..

1) I agree, the assignment operator should not be const. I now changed it.
However, it still erases the last element.

2) If I do vector::erase (it), and 'it' points to "cac", then it shouldn't
point to "cac" after erasing it. It still points to "cac" even after changing
the assignment operator..

All suggestions are welcome,
Thank You

Share this post


Link to post
Share on other sites
Quote:
Original post by fpsgamer
I see two problems:

(1) AALIAS operator=(const AALIAS &a) const, Why is your assignment operator a const member function? It modifies the object, does it not?

(2) The return value from std::vector::erase() is the element after the last element erased.



(1) Further more, I believe the assignment function should be better looking like this: Type& Type::operator=(const Type& object)

Share this post


Link to post
Share on other sites
Quote:
Original post by IndrekSnt
Thank You for Your reply..

1) I agree, the assignment operator should not be const. I now changed it.
However, it still erases the last element.

2) If I do vector::erase (it), and 'it' points to "cac", then it shouldn't
point to "cac" after erasing it. It still points to "cac" even after changing
the assignment operator..

All suggestions are welcome,
Thank You


I think You probably need to assign the iterator back iter = yourVectorName.erase(iter);

Share this post


Link to post
Share on other sites
Quote:
Original post by IndrekSnt
I agree, the assignment operator should not be const. I now changed it.


Can you post the new definition?

Share this post


Link to post
Share on other sites
AALIAS operator=(const AALIAS &a);

AALIAS AALIAS::operator=(const AALIAS &a) {
AALIAS aa;
aa.Key = a.Key;
aa.Command = a.Command;
return aa;
}




@daviddiligent: Sry, my sentence lost its meaning. What I ment was that vector::erase (it) returns an iterator to "cac".

Thank You

Share this post


Link to post
Share on other sites
Quote:
Original post by IndrekSnt
Thank You for Your reply..

1) I agree, the assignment operator should not be const. I now changed it.
However, it still erases the last element.

2) If I do vector::erase (it), and 'it' points to "cac", then it shouldn't
point to "cac" after erasing it. It still points to "cac" even after changing
the assignment operator..

All suggestions are welcome,
Thank You


I have questions here.
1> How do you know that it erased the last element? 2> How do you make the iter points to "cac".
If it always erased the last element, doesn't that mean the find() only returns the iterator to the last element - 1? If it did, how did you make your iter points to "cac" ?

By the way, I think the better version of the "=" would be:

AALIAS& AALIAS::operator=(const AALIAS& ob)
{
key = ob.key;
command = ob.command;
return *this;
}

Share this post


Link to post
Share on other sites
By the way, if that is all your operator= does, you don't need to write one yourself at all. The compiler would create one that does the same for you automatically, except without errors :)

Share this post


Link to post
Share on other sites

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

If you intended to correct an error in the post then please contact us.

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this