Sfml: declare a destructor for sf::NonCopyable

Created on 6 Aug 2016  路  8Comments  路  Source: SFML/SFML

I think there must be a destructor with protected access specifier in NonCopyable. current implementation works, and it is fine, but think of a begginer, who derive a class from NonCopyable by public inheritance, then he uses a pointer to base class! but if destructor is protected, it is safer, because he cannot use delete on the pointer. but in this way he can call delete, and result is a memory leak.
I know there is no reason to do that, but it is possible. I think declaring a destructor, satisfy the rule that make interface hard to use incorrectly.

bug sfml-system accepted

Most helpful comment

No I think @shayanaminnjad wants to add a protected non-virtual destructor. This will prevent delete being called on a NonCopyable* which will leak resources any subclass has allocated.

This is exactly what boost does for instance:
http://www.boost.org/doc/libs/1_61_0/boost/core/noncopyable.hpp

All 8 comments

I assumed you meant adding a _virtual_ dtor, but I disagree: no one sane would keep a sf::NonCopyable* around. There's no value to it. Also, it would imply that every class inheriting from sf::NonCopyable have a vtable and this would be useless in some situations.

No I think @shayanaminnjad wants to add a protected non-virtual destructor. This will prevent delete being called on a NonCopyable* which will leak resources any subclass has allocated.

This is exactly what boost does for instance:
http://www.boost.org/doc/libs/1_61_0/boost/core/noncopyable.hpp

No, virtual is not appropriate for NonCopyable class! i meant a non virtual protected destructor! any way the class will declare an implicit destructor, so there is no use in it, to be public! if you make it protected, that would be safer

Ha, sorry! For some reason my brain skipped over the protected in your description. Go figure...

Anyway, now I see your point.

I think it's also important to point out that sf::nonCopyable shouldn't really be used in user code, since C++11 introduced better approaches. But I guess for all those poor people who can't switch to a C++11 compiler, we could update our code somewhat.

For future issues like that, I recommend to follow the contribution guidelines and start a discussion first on the forum

@eXpl0it3r I don't think it is about poor people, many people read a book like dietel dietel c++, then they may read scott meyers effective c++, and then want to jump into programming world by making games, and they find SFML. so i think some one maybe is reading for 3 years about programming, and still he is not aware of delete keyword for functions.

It is a bit pedantic, but I agree with op that its better for something like sf::nonCopyable to have a protected non-virtual destructor. Anyone who does not have C++11 available can reuse this one instead of making their own.

class A : public sf::nonCopyable {
 public:
  virtual ~A();
};

class B : public A {
 public:
  virtual ~B();
};

void foo() {
  B* b = new B();
  delete b; //ok

  A* a = new B()
  delete a; //ok

  sf::nonCopyable* nc = new B();
  delete nc; //Oops! Should be a compiler error.
}

Yes, deleting a sf::nonCopyable* is silly, but it doesn't hurt to add this extra bit of safety. Let's do it.

Was this page helpful?
0 / 5 - 0 ratings