std::vector .begin() throwing error

[Cube777]

Hi guys, I have been trying to use the .begin() function of a std::vector inside a class, but it throws an error. I have been using the exact same line of code inside other member functions of my class without any problems. It throws an error as soon as I call .begin() or .end(). The script:

bool AlienManager::CollisionCheck(sf::FloatRect blastradius)
{
	bool _intersects = false;
	std::vector<Object_Alien>::iterator itr = _objects.begin();
	while (itr != _objects.end())
	{
		bool temp = blastradius.intersects(itr->GetGlobalBounds());
		if (temp)
		{
			itr = Remove(itr);
			_intersects = true;
		}
		else
			itr++;
	}
	return _intersects;
}

The _object variable is of std::vector<Object_Alien> type, and it is only in this function where it throws this error. _objects is never deleted so I have no idea why it is throwing the error. Also I thought .begin() had a no-throw guarantee. The error I'm receiving from VS is as follows:

Unhandled exception at 0x00a544f4 in Space Invaders.exe: 0xC0000005: Access violation reading location 0xcdcdcdd5.

Is there anything else I can try? Thanks for any help in advance!

EDIT: Also I should mention I tried swapping out std::vector for std::list yet I get the same error

[poteto]

I might be wrong, but as I remember iterators should be used like:
(*itr)->

Why do you use "Remove" instead with "_objects.erase(itr);"?

Are you multi-threading?

Also, don't do "_..." names, they can cause name clashes, do "..._".

[BasV]

This code seems ok to me, maybe something is going wrong in your Remove function?

[BasV]

The iterator is used correctly in this case, (*itr)-> is useful when you have a array of pointers ;)

Unless of course Object_Alien is a typedef of a pointer type.

[Cube777]

Thanks for the quick replies!

@poteto The std::vector contains the classes themselves, not pointers to them because I read somewhere that to store raw pointers in containers is a bad idea (please correct me on this if I'm wrong). Also I read that calling .erase() doesn't call the destructor of the class contained in the vector, so Remove() first deletes the class then I'm using .erase() to remove the objects from the vector.

@BasV I also thought so but from previous testing the Remove() function seems to be working fine. The classes contained in the vector are allocated with the new function, so the Remove function is as follows:

std::vector<Object_Alien>::iterator AlienManager::Remove(std::vector<Object_Alien>::iterator itr)
{
	_currentScore += itr->GetScore();
	delete &itr;
	return _objects.erase(itr);
}

I just thought about this, is the Remove function already deleting the objects from the vector before using .erase()?

EDIT: Ok I reverted back to to the vector storing the pointers to the classes, so the remove function is now the following:

std::vector<Object_Alien*>::iterator AlienManager::Remove(std::vector<Object_Alien*>::iterator itr)
{
	_currentScore += (*itr)->GetScore();
	delete (*itr);
	return _objects.erase(itr);
}

[BasV]

You shouldn't delete &itr. First of all, you're deleting a pointer to the iterator, and you never allocated that yourself.

Secondly, if you want to delete your old object, you probably don't need to. If it's not a pointer, erase will do everything for you. Unless you have a specific function that does cleaning up, you can call that now.

If you never allocated your Object_Alien, you don't need to delete it.

I presume you initialize your Object_Aliens by doing something like:

Object_Alien ob;

_objects.push_back(ob);

In that case, just calling

_objects.erase(itr);
is fine!


edit: I overlooked you saying that they are allocated with the new statement. When you place such objects in a vector, a copy is made. So your original allocated data is now lost and you can't delete that anymore. You need to store the pointers in an array, and delete those:

std::vector<Object_Alien*> _objects;

[ne555]

> Unhandled exception: Access violation reading location 0xcdcdcdd5.
> Also I thought .begin() had a no-throw guarantee.
I'll concede that the message can be confusing.
The problem is not a throw that is not being catch, the problem is trying to read memory that you don't own. (in your case seems to be produced by those bad deletes)

The trhow case (like in .at() ) would be something like

gdb wrote:

terminate called after throwing an instance of '...'
Program received signal SIGABRT, Aborted.

> You need to store the pointers in an array, and delete those:
Better yet, store intelligent pointers so you don't need to delete those.
As a rule, if you have to write delete you are probably doing something wrong

[Cube777]

Ok I tried avoiding using delete so I rewrote the code so that I simply declare a class without using new and then add it to the _objects, but I noticed something. Object_Alien is merely a superclass, all the classes I add to _objects are all derived classes of Object_Alien, and although I declared the main functions needed in the derived classes as virtual, it still calls the functions defined in Object_Alien, not the derived class. How can I fix this? Thanks for all the help so far!

[MiiNiPaa]

You have a slicing problem. After you shore Child object in container of Parents, the object stored will be of class Parent! All additional info will be lost.

In this case you need container of [smart]pointers, or references (reference_wrappers to be precise, as you cannot store references in containers directly)

[Cube777]

@MiiniPaa Thank you for the explanation, first time getting into OOP :)

After tedious debugging I found the error... I forgot to initialize the _alienManager variable... Thank you for the help guys, happy holidays!