Jump to content

  • Log In with Google      Sign In   
  • Create Account

Is it acceptable to call a destructor?


Old topic!
Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.

  • You cannot reply to this topic
40 replies to this topic

#1 Akashi   Members   -  Reputation: 268

Like
0Likes
Like

Posted 18 October 2012 - 10:39 AM

I read into it a bit, and the little information I got was from this source:

You can use a destructor explicitly to destroy objects, although this practice is not recommended.


I have an enemy object that is destroyed upon being intersected with a projectile object. Instead of making a bunch of if statements in a for loop outside the object, wouldn't it be easier and cleaner to make the enemy object just destroy itself if it detects colliding with a projectile? Would it be okay, according to conventions, to call ~enemy INSIDE the enemy class, rather than having to code outside the enemy class?

Edited by StoneMask, 18 October 2012 - 10:40 AM.


Sponsor:

#2 greenvertex   Members   -  Reputation: 510

Like
6Likes
Like

Posted 18 October 2012 - 10:49 AM

While you can certainly do it, don't. Calling a class destructor inside the object itself basically always leads to undefined behavior. No other part of your code is aware that you've just invalidated every reference to that object. Are you 100% sure none of those references will be used after destruction?

You might want to review your collision code in general. While it might be "easier" to explicitly invoke a destructor it's probably a sign of code smell and should be dealt with sooner rather than later. If you post the code, we might be able to offer some advice on how better to do your collision.

#3 Brother Bob   Moderators   -  Reputation: 8631

Like
14Likes
Like

Posted 18 October 2012 - 10:49 AM

How do you ensure that the destructor isn't called again when the object is actually about to be destroyed (for example goes out of scope, you release the memory by calling delete, or removing the object from an std::vector)?

What's wrong with having the outside logic taking care of that? If the enemy destroys itself, then that kind of implies that the enemy itself is responsible to handling the collision with projectiles. Isn't that a job for some outside component to handle collisions, like a physics component or something?

#4 ARCGames   Members   -  Reputation: 308

Like
0Likes
Like

Posted 18 October 2012 - 11:06 AM

In general, it's poor form to directly call a destructor (save for the case where you're calling a parent class destructor from a child class desctructor).

Honestly I've never heard of anyone doing this and I can think of a lot of things that could go wrong. For instance, if after your ~Enemy() call, you attempt to manipulate the object (either outside the class or inside the class itself), you'll have an invalid pointer.

For instance:

[source lang="cpp"]... // NOTE: Assumes you have a vector/array of enemies and a pointer to the projectile. for( int i = 0; i < numEnemies; ++i ) { if( checkCollision( enemies[i], pProjectile ) ) { // Destructor will be called right here in onCollision(). enemies[i].onCollision(pProjectile); } // Now you have an invalid reference to your object and this call will produce only sadness (crash or undefined behavior). enemies[i].someOtherFunction(); }...[/source]
My code makes a picture of Sephiroth!

#5 frob   Moderators   -  Reputation: 22787

Like
12Likes
Like

Posted 18 October 2012 - 11:08 AM

Don't call the destructor. It will be called automatically.

This is the case 99.9999999% of the time.


For the 0.00000001% of the time:

There is only one place you should ever call a destructor manually, and that is if you are using something called a "placement new".

Using a "placement new" is a fairly advanced topic that is almost never done in practice.

Check out my book, Game Development with Unity, aimed at beginners who want to build fun games fast.

Also check out my personal website at bryanwagstaff.com, where I write about assorted stuff.


#6 frob   Moderators   -  Reputation: 22787

Like
11Likes
Like

Posted 18 October 2012 - 11:12 AM

In general, it's poor form to directly call a destructor (save for the case where you're calling a parent class destructor from a child class desctructor).

No, don't even do it then. The language has guarantees that when a destructor is called, the base class destructors are automatically called as well. Do not call them manually during your destructor.

Check out my book, Game Development with Unity, aimed at beginners who want to build fun games fast.

Also check out my personal website at bryanwagstaff.com, where I write about assorted stuff.


#7 Felix Ungman   Members   -  Reputation: 1066

Like
1Likes
Like

Posted 18 October 2012 - 11:41 AM

Also, keep in mind the difference between allocation/deallocation and construction/destruction. If the enemy holds projectile objects (e.g. an std::vector<projectile>) you don't need to do anything extra, all destructors are called automatically. But if it holds object *pointers* (e.g. std::vector<projectile*>) you need to decide on ownership. It's usually a good idea keep allocation and deallocation responsibility together. If it's the enemy object that allocates and stores projectiles, it should likely deallocate them too in its destructor.

[edit]

Ah, reading the OP I see the enemy doesn't hold projectiles. However, the principle that keeping allocation and deallocation together is still good advice here. And in any case, neither the birth and death of an object instance should be controlled by that instance itself. The former is just counter-intuitive and the latter too controversial.

Edited by Felix Ungman, 18 October 2012 - 11:53 AM.

openwar  - the real-time tactical war-game platform


#8 BeerNutts   Crossbones+   -  Reputation: 3018

Like
4Likes
Like

Posted 18 October 2012 - 11:50 AM

You also have to realize, when you figure out the enemy is dead, and needs to be removed, it will also have to be removed from the list of current enemies. So, whether this list is std::vector, std::list, linked list, or array, you're going to have to ensure that enemy isn't updated any more. Thus, you will HAVE to do something outside of the enemy object to handle it's removal. Like this:

Have your Enemy::Update() function return the status of the enemy (alive, dead, whatever), and remove it then:
// assuming EnemyList is a std container
for (int i = 0; i < EnemyList.size(); ) {
  if (EnemyList[i].Update() == ENEMY_DEAD) {
    // If The list stores pointers, you'll have to call delete here
    EnemyList.erase(EnemyList.begin() + i);
  }
  else {
    ++i;
  }
}

My Gamedev Journal: 2D Game Making, the Easy Way

---(Old Blog, still has good info): 2dGameMaking
-----
"No one ever posts on that message board; it's too crowded." - Yoga Berra (sorta)

#9 Álvaro   Crossbones+   -  Reputation: 13934

Like
1Likes
Like

Posted 18 October 2012 - 01:20 PM

[Waits for someone to post some std::remove_if invocation in response to BeerNutts's code...]

#10 Brother Bob   Moderators   -  Reputation: 8631

Like
2Likes
Like

Posted 18 October 2012 - 01:43 PM

[Waits for someone to post some std::remove_if invocation in response to BeerNutts's code...]

That was my first reaction as well:
EnemyList.erase(
	std::remove_if(
		std::begin(EnemyList),
		std::end(EnemyList),
		[](Enemy &e) {return e.Update() == ENEMY_DEAD;}),
    std::end(EnemyList));


#11 AlexB.hpp   Members   -  Reputation: 201

Like
-3Likes
Like

Posted 18 October 2012 - 02:31 PM

Ofc you can use it that way. It will looks smth like this:
We run loop on an array of some objects to check some abstract requirement.
#include <stdio.h>
#include <list>
class A {
public:
  A() {
	printf("A:constructor.\n");
  }
  ~A() {
	printf("A:destructor.\n");
  }
};
int main() {
  std::list<A*> objects;
  printf("Allocate 10 objects of A\n");
  for (int i = 0; i < 10; i++)
	objects.push_back(new A());

  printf("Time to die\n");
  int list_size = objects.size();
  for (int i = 0; i < list_size; i++) {
	A* deadbeef = *(objects.begin());
	objects.erase(objects.begin());
	delete deadbeef;
  }
  printf("Elements in list: %d\n", objects.size());
}

Output:
Allocate 10 objects of A
A:constructor.
A:constructor.
A:constructor.
A:constructor.
A:constructor.
A:constructor.
A:constructor.
A:constructor.
A:constructor.
A:constructor.
Time to die
A:destructor.
A:destructor.
A:destructor.
A:destructor.
A:destructor.
A:destructor.
A:destructor.
A:destructor.
A:destructor.
A:destructor.
Elements in list: 0

But this is now clever 'cause memory allocation is kinda slow operation. Much better if we will save memory for such type of object and use it letter.

#include <stdio.h>
#include <list>
class A {
public:
  A() {
	printf("A:constructor.\n");
  }
  ~A() {
	printf("A:destructor.\n");
  }
  void Clear() {
  }
};
class AAlloc {
  std::list<A*> _cache;
public:
  A* Alloc() {
	if (!_cache.empty()) {
	  A* deadbeef = _cache.front();
	  _cache.erase(_cache.begin());
	  deadbeef->Clear();
	  return deadbeef;
	}
	return new A();
  }
  void Dealloc(A* obj) {
	_cache.push_back(obj);
  }
  ~AAlloc() {
	std::list<A*>::iterator itr = _cache.begin();
	for ( ; itr != _cache.end(); ++itr) {
	  delete *itr;
	}
  }
};
int main() {
  std::list<A*> objects;
  AAlloc allocator;
  printf("Allocate 10 objects of A\n");
  for (int i = 0; i < 10; i++)
	objects.push_back(allocator.Alloc());

  printf("Time to die\n");
  {
	std::list<A*>::iterator itr = objects.begin();
	for ( ; itr != objects.end(); ++itr) {
	  allocator.Dealloc(*itr);
	}
	objects.clear();
  }
  printf("One more allocation of 10 objects\n");
  for (int i = 0; i < 10; i++)
	objects.push_back(allocator.Alloc());
  printf("IT\'S A GOOD DAY TO DIE!\n");
  {
	std::list<A*>::iterator itr = objects.begin();
	for ( ; itr != objects.end(); ++itr) {
	  allocator.Dealloc(*itr);
	}
	objects.clear();
  }
  printf("Elements in list: %d\n", objects.size());
}


New output:
Allocate 10 objects of A
A:constructor.
A:constructor.
A:constructor.
A:constructor.
A:constructor.
A:constructor.
A:constructor.
A:constructor.
A:constructor.
A:constructor.
Time to die
One more allocation of 10 objects
IT IS A GOOD DAY TO DIE!
Elements in list: 0
A:destructor.
A:destructor.
A:destructor.
A:destructor.
A:destructor.
A:destructor.
A:destructor.
A:destructor.
A:destructor.
A:destructor.

It's maybe looks pretty tricky but the main reason is speed up application.

Edited by AlexB.hpp, 18 October 2012 - 04:48 PM.

C x 2 C x o C x C f nice C x C s C x C c

#12 Ravyne   GDNet+   -  Reputation: 8194

Like
1Likes
Like

Posted 18 October 2012 - 03:02 PM

It's probably preferable in this case to do some kind of mark-and-sweep pattern (As Brother Bob showed). Its possible to call a destructor safely, but only in very specific circumstances.

#13 BeerNutts   Crossbones+   -  Reputation: 3018

Like
3Likes
Like

Posted 18 October 2012 - 03:33 PM


[Waits for someone to post some std::remove_if invocation in response to BeerNutts's code...]

That was my first reaction as well:
EnemyList.erase(
	std::remove_if(
		std::begin(EnemyList),
		std::end(EnemyList),
		[](Enemy &e) {return e.Update() == ENEMY_DEAD;}),
	std::end(EnemyList));


IMO, this kind of code shouldn't be introduced in a "For Beginner's" forum. It's likely to add confusion rather than help. I try to keep things simple when replying here.
My Gamedev Journal: 2D Game Making, the Easy Way

---(Old Blog, still has good info): 2dGameMaking
-----
"No one ever posts on that message board; it's too crowded." - Yoga Berra (sorta)

#14 ATC   Members   -  Reputation: 551

Like
0Likes
Like

Posted 18 October 2012 - 03:48 PM

Not trying to hijack the thread here but placement new isn't useless or "almost never used". Lots of large games need their own memory manager with a smart point/reference system and want their objects to be allocated in their memory management system's contiguous, managed heap. Thus placement new operator is used to put them there rather than in statically allocated bits of memory. I've done this before myself.

But I agree that use of placement new is not common; it's simply not necessary most of the time. But in large, AAA games that can be dealing with several GB of memory it's often a good idea to have a memory manager and use placement new.
_______________________________________________________________________________
CEO & Lead Developer at ATCWARE™
"Project X-1"; a 100% managed, platform-agnostic game & simulation engine


Please visit our new forums and help us test them and break the ice!
___________________________________________________________________________________

#15 Brother Bob   Moderators   -  Reputation: 8631

Like
8Likes
Like

Posted 18 October 2012 - 04:36 PM



[Waits for someone to post some std::remove_if invocation in response to BeerNutts's code...]

That was my first reaction as well:
EnemyList.erase(
	std::remove_if(
		std::begin(EnemyList),
		std::end(EnemyList),
		[](Enemy &e) {return e.Update() == ENEMY_DEAD;}),
	std::end(EnemyList));


IMO, this kind of code shouldn't be introduced in a "For Beginner's" forum. It's likely to add confusion rather than help. I try to keep things simple when replying here.

And in my opinion, a loop with strange loop increment conditions and logic is not better that code that has well defined parameters and function calls. I can separate the remove and the erase calls into separate statements if you want to simplify and understand the logic behind it, something that's going to be very difficult with your loop since the two are tightly coupled with each other.

Removed some namespace qualifiers as well if you want to clean up the code a bit.
auto pred = [](Enemy &e) {return e.Update() == ENEMY_DEAD;};
auto last = remove_if(begin(EnemyList), end(EnemyList), pred);

EnemyList.erase(last, end(EnemyList));


It's a matter of taste and what you already know and what you want to learn. Your code may be simpler for you but my code is certainly simpler for me. Both codes have their details you have to know about to understand them. For example, your code requires knowledge about how iterators work when removing elements from a container you iterate over, while my code requires knowledge of function objects of some kind (lambda expressions in my particular example, but you can use functors or just plain function pointers as well). On that topic, I can argue that, in general, knowing major things such as lambda expressions and standard library functions is more important that knowing mostly irrelevant details such as how to adjust a iterator once you have removed an element from a container. I call it irrelevant since it mostly occurs in situations where the standard library already has a function for it.

Just saying that "easy" is in the eye of the beholder.

#16 AlexB.hpp   Members   -  Reputation: 201

Like
0Likes
Like

Posted 18 October 2012 - 04:47 PM

auto pred = [](Enemy &e) {return e.Update() == ENEMY_DEAD;};
auto last = remove_if(begin(EnemyList), end(EnemyList), pred);

EnemyList.erase(last, end(EnemyList));

Too many C++0x. I guess it's difficult for novices. Don't you think so?

But however your code show me some interesting points. Thanks
C x 2 C x o C x C f nice C x C s C x C c

#17 Brother Bob   Moderators   -  Reputation: 8631

Like
2Likes
Like

Posted 18 October 2012 - 05:19 PM


auto pred = [](Enemy &e) {return e.Update() == ENEMY_DEAD;};
auto last = remove_if(begin(EnemyList), end(EnemyList), pred);

EnemyList.erase(last, end(EnemyList));

Too many C++0x. I guess it's difficult for novices. Don't you think so?

But however your code show me some interesting points. Thanks

Both yes and no.

Yes, the whole solution is too much to digest for a beginner. For a beginner of programming that is, but not necessary for a beginner of C++ that has some basic experience with other languages.

No, I don't think the C++11 parts of my solution makes it difficult. Very much the opposite in fact. Consider the variable declarations that would be necessary for pred and last if it wasn't for the auto keyword. Consider the beauty of specifying code directly where it is used instead of somewhere else.

With reservation for some syntax errors of course, even VS2010 with it's very limited C++11 support can compile it. There's really nothing exceptional and brand new about it.

#18 Akashi   Members   -  Reputation: 268

Like
0Likes
Like

Posted 18 October 2012 - 07:50 PM

Yeah, I have used a similar procedure for collisions and deallocation before. Something like the following:

// at some point in code
projectile missile;
new enemy[num];

// check for enemy collisions
for (short i = 0; i < num; i++)
{
	 enemy[i].move();

	 if (enemy[i].collide(missile) == true)
		 delete [i] enemy;
}


Please note that this is highly abridged. My game's world is actually based around a character array, so I don't even have to pass the object, just a character value.

Thanks for all your help though; I understood when BeerNutts mentioned the increment step being messed up because of calling the destructor.

Edited by StoneMask, 19 October 2012 - 08:34 AM.


#19 SuperVGA   Members   -  Reputation: 1118

Like
0Likes
Like

Posted 19 October 2012 - 03:30 AM

How do you ensure that the destructor isn't called again when the object is actually about to be destroyed (for example goes out of scope, you release the memory by calling delete, or removing the object from an std::vector)?

What's wrong with having the outside logic taking care of that? If the enemy destroys itself, then that kind of implies that the enemy itself is responsible to handling the collision with projectiles. Isn't that a job for some outside component to handle collisions, like a physics component or something?

Exactly what Brother Bob says; why would you allow an object to self destruct? If the enemy class had a bool collision(vec3 from_pt, vec3 to_pt) and returned true
for collision, you could handle destruction just after handling the bullet physics.
To separate your subsystems collision, game logic, models etc, you'd probably want to go with those bloody iterations, flags and internal messaging systems anyways,
but it's "nicer" to delete or std::pop() / remove than it is to selfdestruct. IMO.

EDIT: Oh, I just noticed that you've taken it a few steps further. I will leave you two to it. :)

Edited by SuperVGA, 19 October 2012 - 03:32 AM.


#20 Akashi   Members   -  Reputation: 268

Like
0Likes
Like

Posted 19 October 2012 - 11:30 AM

Are you saying I put it a step further, lol?

I'm unfamiliar with the syntaxes some of you are giving me, or I don't really find them very clean/readable (no offense). So to me it looks like I'm doing less work or I'm lacking in some area of efficiency or functionality. I kind of think that way whenever I run into an unfamiliar syntax or code that looks intimidating to read through.

Compared to the methods that have been discussed in the thread compared to my posted way of doing it, which is the best in terms of efficiency and cleanliness? In C++ or something similar, please. Or even pseudocode!

Edited by StoneMask, 19 October 2012 - 11:36 AM.





Old topic!
Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.



PARTNERS