Welcome, Guest. Please login or register. Did you miss your activation email?

Author Topic: Virtual Destructors?  (Read 8021 times)

0 Members and 1 Guest are viewing this topic.

TheComet

  • Newbie
  • *
  • Posts: 12
    • View Profile
Virtual Destructors?
« on: May 30, 2013, 08:50:42 pm »
Hey,

I'm very new to SFML, so I don't know the guts of it.

However, I couldn't help but notice that no destructors in any of the classes are declared virtual, which could cause memory leaks when trying to inherit them, and using a base pointer to address the object.

This is an example of what I mean:

class cDerivedTexture : public sf::Texture
{
public:
   cDerivedTexture( void );
   ~cDerivedTexture( void );
private:
   // stuff, not really important
};


// Then, later on...


// make a new texture
sf::Texture* myTexture = new cDerivedTexture();

/* --- SNIP --- */

// now delete the texture
delete myTexture; // Oh oh, destructor of sf::Texture isn't called!

I'm going to assume the destructor sf::Texture::~Texture() plays an important role in freeing up memory of loaded images, but in the above example, it isn't called when deleting the derived texture class.

The fix would be to make it virtual:

// The SFML texture class
class Texture
{
   // destructor of sf::Texture
   virtual ~Texture();

   // other stuff
}

Any thoughts or ideas?

TheComet
« Last Edit: May 30, 2013, 10:11:01 pm by Laurent »

Laurent

  • Administrator
  • Hero Member
  • *****
  • Posts: 32504
    • View Profile
    • SFML's website
    • Email
Re: Virtual Destructors?
« Reply #1 on: May 30, 2013, 08:55:15 pm »
And don't you think that it is a good indication that these classes are not meant to be inherited, and, above all, not used polymorphically? ;)

You can't prepare all your classes for all potential usage. Well, you can, but it's a crazy strategy. You have to define only what is relevant, and what is needed for the intented usage. That's what I do in SFML.

Specifically, sf::Texture should never be derived publicly and used polymorphically. It has no virtual function, so there's no point doing that.
Laurent Gomila - SFML developer

TheComet

  • Newbie
  • *
  • Posts: 12
    • View Profile
Re: Virtual Destructors?
« Reply #2 on: May 30, 2013, 09:48:26 pm »
I wanted to write a texture manager class which would keep track of all loaded textures. There are numerous cases where the same texture is used multiple times, and I wanted to write a class which would keep track of all of the loaded textures and prevent duplicates in memory. For this to happen, each texture needs to store some additional data about it (perhaps the path it was loaded from).

My thoughts were to write another class inheriting the sf::Texture class in order to add more methods and attributes to it, which would look somewhat like this:

class cTexture : public sf::Texture
{
   // inherits everything from sf::Texture

private:
   
   // stores the file path the texture was loaded from so it can be compared with other textures in the future
   std::string filePath;
};

I'm unsure if this is the correct way to approach this, any feedback is appreciated. :)

Nexus

  • SFML Team
  • Hero Member
  • *****
  • Posts: 6286
  • Thor Developer
    • View Profile
    • Bromeon
Re: Virtual Destructors?
« Reply #3 on: May 30, 2013, 09:54:16 pm »
For this to happen, each texture needs to store some additional data about it (perhaps the path it was loaded from).
This does not imply inheritance. You can create a class that contains a sf::Texture member and other data (composition).

You could also have a look at thor::ResourceCache (API doc, tutorial) which provide what you need -- namely a way to avoid duplicate loading. However, I have meanwhile made the experience that this is often overkill, a simple container like a map is already enough for cases where you have clearly-defined ownership semantics.
Zloxx II: action platformer
Thor Library: particle systems, animations, dot products, ...
SFML Game Development:

TheComet

  • Newbie
  • *
  • Posts: 12
    • View Profile
Re: Virtual Destructors?
« Reply #4 on: May 31, 2013, 12:48:22 am »
Thanks for the link, thor is going to help a great deal with my project. :)

kralo9

  • Newbie
  • *
  • Posts: 44
    • View Profile
    • Email
Re: Virtual Destructors?
« Reply #5 on: May 31, 2013, 09:50:23 am »
Why don't try something like
class Texture{
public:
//some accessors etc
private:
sf::Texture _texture;
std::string _path;
};
 

Store this in a std::vector and you can easily load textures and check if a path is alteady loaded
I'm totally exhausted. I've got 3 children and no money! Why can't I have no children and 3 money? - Homer S.

JayArby

  • Jr. Member
  • **
  • Posts: 68
    • View Profile
Re: Virtual Destructors?
« Reply #6 on: May 31, 2013, 11:13:56 pm »
It makes a lot more sense to inherit from a Texture than to "contain" it and then duplicate its entire API. Seriously, the destructors should just be virtual. How much of a performance hit could it possibly be? And this is not trying to anticipate every possible situation--it's trying to anticipate the most basic, common of requirements.

FRex

  • Hero Member
  • *****
  • Posts: 1845
  • Back to C++ gamedev with SFML in May 2023
    • View Profile
    • Email
Re: Virtual Destructors?
« Reply #7 on: June 01, 2013, 12:11:51 am »
You don't have to duplicate API and deleting derived sf::Texture through a base pointer is artificial problem, there is literally no reason to do it, all other classes that have any virtual methods have virtual destructors.
Back to C++ gamedev with SFML in May 2023

Laurent

  • Administrator
  • Hero Member
  • *****
  • Posts: 32504
    • View Profile
    • SFML's website
    • Email
Re: Virtual Destructors?
« Reply #8 on: June 01, 2013, 09:05:33 am »
Quote
It makes a lot more sense to inherit from a Texture than to "contain" it and then duplicate its entire API. Seriously, the destructors should just be virtual. How much of a performance hit could it possibly be? And this is not trying to anticipate every possible situation--it's trying to anticipate the most basic, common of requirements.
If you inherit from sf::Texture, then you have to keep the derived type in order to use the extra functions, which means that there's no polymorphism and no issue with the destructor.

But anyway, this is really not the right design. Look at some other popular C++ libraries, can you find one where all the destructors are virtual?
Laurent Gomila - SFML developer

JayArby

  • Jr. Member
  • **
  • Posts: 68
    • View Profile
Re: Virtual Destructors?
« Reply #9 on: June 01, 2013, 09:01:04 pm »
You're right, there's really no polymorphism, but that isn't the point. It's really just a way to add features to the type. In a language like ruby, you could do this without inheritance. In C++, that isn't possible without wrapping the class. If you are a purist about it, this a "wrong" usage of inheritance. But in practical terms, it works and it makes sense and I don't see any disadvantage in it.

Laurent

  • Administrator
  • Hero Member
  • *****
  • Posts: 32504
    • View Profile
    • SFML's website
    • Email
Re: Virtual Destructors?
« Reply #10 on: June 01, 2013, 09:14:40 pm »
What's the point of a virtual destructor if there's no polymorphism? And what's the point of polymorphism if there's no virtual function? There's no point.

You write stuff when it makes sense from a design point of view. Not just because "there's no disadvantage to it" ;)
Laurent Gomila - SFML developer

JayArby

  • Jr. Member
  • **
  • Posts: 68
    • View Profile
Re: Virtual Destructors?
« Reply #11 on: June 01, 2013, 09:16:12 pm »
Well I thought I had explained the point of it: it's essentially encapsulation. The virtual destructor is to avoid memory leaks, as pointed out.

Laurent

  • Administrator
  • Hero Member
  • *****
  • Posts: 32504
    • View Profile
    • SFML's website
    • Email
Re: Virtual Destructors?
« Reply #12 on: June 01, 2013, 09:20:13 pm »
But why would there be a memory leak if you don't delete the instance polymorphically? Since there's no point doing that, there's no problem.

I don't want to write code that would make people's crazy code work fine. I'd rather suggest them how to write better code.

And public inheritance is the worst possible level of encapsulation. In fact you don't encapsulate anything when you inherit publicly from a class. When you wrap it into a container class, however, you do encapsulate it.
Laurent Gomila - SFML developer

MorleyDev

  • Full Member
  • ***
  • Posts: 219
  • "It is not enough for code to work."
    • View Profile
    • http://www.morleydev.co.uk/
Re: Virtual Destructors?
« Reply #13 on: June 03, 2013, 02:49:56 pm »
This Is-A/Has-A model is a broken way to look at inheritance. It results in overcomplicated designs. A good way to view it is when you inherit you duplicate the responsibilities. Inheritance is a means through which one class can duplicate and extend the responsibilities of another class.

Sometimes this is good, sometimes you want to duplicate and extend responsibilities. But most of the time you do not want to do this. Code is always at it's simplest and cleanest when it has the fewest responsibilities, so inheritance will almost always make your design more complicated.

This is a situation where the code is made so much simpler by encapsulation, by keeping the myriad of responsibilities of the sf::Texture from polluting your class.
UnitTest11 - A unit testing library in C++ written to take advantage of C++11.

All code is guilty until proven innocent, unworthy until tested, and pointless without singular and well-defined purpose.

JayArby

  • Jr. Member
  • **
  • Posts: 68
    • View Profile
Re: Virtual Destructors?
« Reply #14 on: June 05, 2013, 07:26:47 pm »
Sorry, I used the wrong term. I didn't mean encapsulation; I meant aggregation.