Jump to content
  • entries
    5
  • comments
    43
  • views
    10,656

The horrible and disgusting "static class"


Mumbles

6,194 views

 Share

It's something I've always been opposed to - creating a class, knowing full well I'll only create one instance of that class, or none at all. I've always been an advocate of the idea "only create classes if you want to create several instances of the class". It's what always kept me away from the idea of using LEO, instead of using the plain C interface.

 

How many "engines" are going to be running inside my game? Just the one, so why make an engine class? Despite saying this, for years I did have some of my own 1-instance classes - usually some sort of linked list (I know these exist in the STL libraries but I'm a bit of a nutter and like to reinvent even basic things). I'd set them up as classes because I wished to hide some data, such as the pointer to the head of the list, so that it was always valid and could never be tampered with accidentally. The private access specifier was great for this, and anything else I never wanted to call/access accidentally.

 

Sadly, these private members always showed up in intellisense and it even offered to autocomplete them for me, only to then tell me that I was accessing a member function that was out of scope. Not hugely serious, as it was only a quick few seconds to replace the bad function call with the right one, but a little irritating nonetheless.

 

 

The lergest problem I was running into was that I noticed I lacked consistency. Half of these list classes I'd designed with the intention of being "static classes" and the other half expected an instance. The was no compelling reason to have chosen either, and was probably determined each time by how much stamp glue I'd been sniffing in the previous hours. I turned to Google in an attempt to standardise my classes and find out in no uncertain terms which was the better way, and then update my classes to reflect this.

 

I soon found though, that really, in the land of C++, neither was really the right option. Static classes were a sort of "hack" solution for Java (which is what we were taught at my uni) to achieve the same thing that C++ achieves with anonymous namespaces. Well, I call them anonymous namespaces because it sounds fancier than unnamed namespaces which is their correct name.

 

Indeed, I'd always known that namespaces, didn't actually have to have a name (even though the word namespace kind of implies it does), but I'd never seen any advantage of them. Since discovering how to use them "properly", I sort of couldn't keep it to myself, so hopefully any other C++ users out there who thought the same as I once did, could learn something here.

 

Here's one of my list classes (header only), as a list. It's one of the simpler lists, which much more resembles an STL linked list, than some of my others do

 

class RaysList
{
public:
RaysList();
~RaysList();
void AddProjectile(RayObject * RayToAdd);
void DeleteProjectile(RayObject * RayToRemove);
void DeleteExpiredProjectiles(void);
void DeleteAllProjectiles(void);
RayObject * GetNextProjectile(void);
RayObject * GetCurrentProjectile(void);
bool EOL(void);
void ToStart(void);
bool IsEmpty(void);
private:
struct ProjectileListEntry
{
	RayObject * AssociatedProjectile;
	ProjectileListEntry * NextListEntry;
};
ProjectileListEntry * ActiveProjectileHead;
ProjectileListEntry * MemToReadFrom;
};

 

This one as it happens, has no statics (decide for yourself how much glue was involved beforehand), and it looks simple enough.

  • There's a constructor to set the two private pointers so they're never pointing to invalid memory (0 being hardcoded to mean end of list, thus is valid)
  • There's a destructor to free any pointers that were allocated, should the list fall out of scope
  • The member functions are all public so they can all be called validly

 

But it can be re-written without a class, whilst still keeping the access specifiers. "How?" You might ask, because it's true that public, protected and private (well, and friend too) are all class access specifiers - they can't be used for structs or namespaces. Well, here's how:

 

(Header first)

 

namespace RaysList
{
void AddProjectile(RayObject * RayToAdd);
void DeleteProjectile(RayObject * RayToRemove);
void DeleteExpiredProjectiles(void);
void DeleteAllProjectiles(void);
RayObject * GetNextProjectile(void);
RayObject * GetCurrentProjectile(void);
bool EOL(void);
void ToStart(void);
bool IsEmpty(void);
}

 

No pointers at all here, but somehow my functions are going to return some. The magic takes place in the definitions (the .cpp file)

 

namespace RaysList
{
namespace
{
	struct ProjectileListEntry
	{
		RayObject * AssociatedProjectile;
		ProjectileListEntry * NextListEntry;
	};
	ProjectileListEntry * ActiveProjectileHead = 0;
	ProjectileListEntry * MemToReadFrom = 0;
}

void AddProjectile(RayObject * RayToAdd)
{
	ProjectileListEntry * NewEntry = (ProjectileListEntry *) malloc (sizeof(ProjectileListEntry));
	NewEntry->NextListEntry = ActiveProjectileHead;
	NewEntry->AssociatedProjectile = RayToAdd;
	ActiveProjectileHead = NewEntry;
}

}

 

The rest of the functions haven't been included, but for this demonstration they don't need to be, this one function shows you how it works.

 

  • At the top of the file (not shown) is a line to include the header file
  • Again, we open a namespace with the same name, but immediately inside this namespace, we create another one, without a name
  • Anything inside the RaysList namespace has total unrestricted access to the secret namespace
  • So, because of this, the pointer declaration to the secret struct in the first line of AddProjectile() is valid
  • But if were to try and declare a pointer to this struct from outside the namespace it wouldn't be valid, and it wouldn't even show up in intellisense, so you wouldn't even be tempted.

 

Even the lack of a destructor doesn't cause a problem, since the pointer variables are declared outside of functions, thus they never fall out of scope... So you don't have any memory leaking.

 

The only reason we might have wanted a class for this concept... Eliminated... I'll never create a "static class" ever again

 

So there you have it. If you want the same effect as the private access modifier, but don't need to create an entire object, there's a way.

 

Sadly, because I learned Java at uni, I always thought this should have been possible, but never knew how, because of course, in Java, everything is an object, even if you're only going to create one instance of a class.

 Share

21 Comments


Recommended Comments

You are doing C at that point. You can code your entire game that way if you like :)

 

The way I see it this is what the singleton pattern is for.

 

Nice article.

Link to comment

Interesting techniuqe and well written.

One little statement of no direct importance for the technique described, but still not correct.

 

But it can be re-written without a class, whilst still keeping the access specifiers. "How?" You might ask, because it's true that public, protected and private (well, and friend too) are all class access specifiers - they can't be used for structs or namespaces. Well, here's how:

 

A class is same thing as a struct with one and only one difference. The class protection is by default private, while its by default public for the struct. The code below is fully valid as the struct and class is essential the same thing.

 

struct MyStruct
{
MyStruct()
:	_prot(0),
	_priv(0)
{}

protected:
int _prot;
void prot() {}

private:
int _priv;
void priv() {}
};

 

Anyway. What's described here is an interesting way of solving the need for statics. So according to this LEO::Draw could be rewritten to something like this (not tested) if one would like to, although this is kind of "hidden" C-programming :)

 

namespace LEO
{
namespace Draw
{
	void SetColor( const TVec3& color );
	void SetColor( const TVec4& color );
	void SetColor( flt red, flt green, flt blue, flt alpha = 1 );
	void SetBlend( int blendmode );
	void Clear( void );

	void Plot( int x, int y );
	void Line( int x1, int y1, int x2, int y2 );
	void Rect( int x, int y, int width, int  height );
	void Image( const Texture& image, int x, int y, int width = -1, int height = -1 );
	void Text( int x, int y, const_str text, ... );
}
}

 

Question

One question about this arrangement. What is gained by using this namespace technique instead of a static class. I can't see in what way this improve things other than the semantics. Have I missed something or is it just about semantics?

Link to comment

Well, I've just learned something too, from the comments... I thought that access specifiers were invalid for structs, and that all struct members had to be public. Apparently not...

 

 

The advantage is probably none in terms of performance. It's just the way I like to work... I've always thought of classes as "collections", and a collection with just one thing always seemed silly to me. It's like having a shelf reserved just for trophies, and then only putting one trophy on it. I'm fine with classes having some static members, but a class with only static members never felt like a "proper" class to me. So I guess it really is just semantics...

 

The advantage to me is that it prevents the private members from showing up in intellisense. Yes there are icons in the autocomplete list which tell you which members are public, and which are not, but I'm guilty of choosing the name that sounds right and then just tabbing it away, without actually checking if it has the right access specifier.

 

 

And I must say, I would prefer that LEO::Draw namespaces arrangement over classes, but that's probably something unique to me. Except for maybe Images. Personally I might have made Image a class, with a Draw method. But points or lines, no way. A surface might be a class, which contains DrawPoint(), DrawLine() etc public functions though.

 

But remember, LEO isn't just built with me in mind - really it should be the people using it, who decide if they prefer the static class or the namespace approach.

Link to comment

It's just the way I like to work.

 

Clearly you can do whatever you like, but I would suggest using more common ways of doing things so 1) Other people aren't confused by your one off way of doing something and 2) You can read other people's code that are following design patterns and such.

 

You should check out singletons if you haven't already. It's sort of the standard in C++ to do what you are trying to do.

Link to comment

Like already said above, this is C.

Like already said above, confusing.

 

I follow this rule:

- Do I need a state? If yes, Class. If no, function (best in a namespace).

- Singleton or static class? The question here is if I want to control the lifetime. If I want to control the lifetime (eg. for memory consumption), I'll use a singleton, if not I'll use a static class.

 

I think what you are looking for in modern C++ is the pimpl idiom. The idea is to hide your implementation details in an object. So for example you could have:

 

namespace YourCompany {
namespace YourProject {
namespace YourLogicalNamespaces {
namespace Details { //Details means implementation detail

class MyObjectPimpl
{
   //Whatever you have as internals...
};

} //Details

class MyObject
{
private:
   MyObjectPimpl m_pimpl;
   //or std::unique_ptr<MyObjectPimpl> m_pimpl; if you want heap, eg. for memory consumption
};

} //YourLogicalNamespaces 
} //YourProject 
} //YourCompany

Link to comment

The pimpl idiom looks almost identical - the only difference being the obvious: classes instead of namespaces. Thus just boiling down to each individuals preference in semantics. Certainly to me, this doesn't look any less confusing.

 

That said, your example hasn't quite made the class MyObjectPimpl totally secret. If this was defined in a header file, then if I included the header file in an external source file, I could write:

YourCompany::YourProject::YourLogicalNamespaces::Details::MyObjectPimpl * APimplObject = new YourCompany::YourProject::YourLogicalNamespaces::Details::MyObjectPimpl();

 

...and I've just got myself an instance of a class that I'm not supposed to be accessing... But I also wouldn't type this by accident :) and it's accidental mistakes I'm trying to protect against. So really, I'm just nitpicking here, it's not really a valid criticism

 

 

But by converting the Details namespace into an unnamed namespace, it's impossible to fully qualify it from outside YourLogicalNamespaces. Thus MyObject can create instances of the pimpl, but no one else can.

 

 

I learned about unnamed namespaces here - the second answer (which builds on the third answer)

 

http://stackoverflow.com/questions/9321/how-do-you-create-a-static-class-in-c

Link to comment

You have created an object that you may create. If you can use it, do it. If you want to make a secret, just provide a lib or a dll. It's just a dataclass, no more no less. Unnamed namespaces are illogical, you create namespaces to define a scope. If you want to make it private as hell, implement as private and set your implementing class as friend.

 

I'm going to ask one simple question: Who is the owner of:

ProjectileListEntry * ActiveProjectileHead = 0;

ProjectileListEntry * MemToReadFrom = 0;

?

Link to comment

Those pointers have no owner. They are akin to bookmarks, with the struct akin to the book's pages - the book doesn't read itself (in my mind, that sort of behaviour denotes a class), instead they book is read by someone. I could set up an object to read the book, but since I'll only have one book reader object I feel it's overkill to set up a class, when a procedural interface will perform almost identically.

 

Instead I use the visible functions to alter the list and traverse through it. They have global lifetime, but with limited scope so I'm not tempted to try and write the pointer values whenever I feel like it - as I'm sure you'd agree, doing that would be loony...

 

 

However please don't think I'm "anti classes". For completeness, the projectile list points to a RayObject. But the RayObject's test for collision is very different based on whether it is a "hitscan" or moving "projectile". The list relies on polymorphism and virtual members to iterate through the list once rather than two lists, one for hitscans and one for projectiles. This can't be done without classes. Plus due to the sheer rate that these objects will be created and destroyed - and due to their "active" nature ("the book reading itself", or "the projectile moving itself") they're definitely suited to classes, and I wouldn't dream of trying to implement these in a procedural way.

 

//Callback prototypes used by this class
//The prototype for the collision test "prefilter" callback. Prefilter means a ray entered a body's AABB, should this body be accurately checked (slow) for collision, or disregarded now?
//This can be used (for example) for a lightning beam, which starts at the firing player's origin, to stop it hitting the player that fired it. Since it would be overkill to try and ungroup that player from the players group just for the duration of the lightning beam collision check
typedef unsigned int (* RayObjectCollisionPrefilterCallback)(const NewtonBody * body,  const NewtonCollision * collision, void * userData);

//The prototype for the collision test "process hit" callback. Process hit is called for a body which was not discarded from the prefilter, and where the ray hit actual geometry (as opposed to entering the AABB, but then completely missing all faces). This allows us to figure out where the collision was in relation to the body's origin, so we can add the "throwing" force in case of explosions
typedef dFloat (* RayObjectCollisionProces****Callback)(const NewtonBody * body, const dFloat * normal, int collisionID, void * userData, dFloat intersetParam);

//A flexible and adaptable struct for when TestForCollision returns. Your own filter callback will decide if the raycast is an "all body" "closest hit" or "first hit" type. However first hit is not recommended for projectiles since there may be others closer, but with a smaller AABB
struct CollisionHitResponse
{
unsigned int NumEntitie****;
Entity * EntityHit;
//unsigned int NumCollisionPoints; //A collision point is only saved if the entity the ray collided with is also saved, so these "count" variables will be the same, making one redundant - in this case, I chose this one.
TVec3 * CollisionPoint;
};

//The projectile class is a ray, rather than a body. It has no NewtonBody, but may have an associated TEntity
//The ray might be "hitscan" (living for only 1 physics tick, for example a laser beam), or it might gradually move forward (like a rocket)
//Was renamed to RayObject almost instantly
//class RayObject : public Entity
class RayObject
{
public:
RayObject(std::string EntityConfigFileName, std::string SceneName = "");
virtual void TestForCollision(void) = 0;//Returns a pointer to a malloc'd CollisionHitResponse struct if a valid collision occured (remember to free it when finished), otherwise returns 0 (and nothing was malloc'd, so nothing needs freeing)
bool RemoveThisRay(void);//After all collisions have taken place, ray objects that have collided are deleted.
bool IsExplosive(void);
void SetDeleteTime(void);//Adds the zombie time to the current time. Once the current time is higher than the delete time, a projectile will be deleted
unsigned int GetDeleteTime(void);
protected:
bool ExplodesOnContact;
bool LifeExpired;
TVec3 Position;
TVec3 Rotation;
virtual void Move(void){return;} //Hitscans can't move so they inherit this definition and just return instantly, projectiles will redefine this member
unsigned int ZombieTime; //How long does this projectile remain drawn once it's expired? Used to prevent the laser and lightning beams from "flickering". All projectiles and most other hitscans will set this to 0. It can only be set during a constructor, and a projectile constructor will forcibly set it to 0 without even attempting to read it from the config file
unsigned int DeleteTime; //Once a ray has it's delete flag marked, this time will be set, once this time is reached it's deleted.
CollisionHitResponse * CollisionHitData;
bool HitDuringUpdate;
void MarkForDeletion(void);
unsigned long long ECB;

//Previously these two were passed as parameters to the TestForCollision function.
//Now they are instead set during the constructor, read from the entity config file. However, it's not possible to users to write their own callback functions. Certain strings will "map" to different callbacks - only the predefined callbacks are valid.
//However it would allow people to create an instant hit shotgun, by using the machinegun callbacks for their shotgun - so that kind of mod-ability will still be present
RayObjectCollisionPrefilterCallback PrefilterCallback;
RayObjectCollisionProces****Callback Proces****Callback;
private:
void InitialiseL1();//Only the correct constructor can call this. Constructors for derived classes can't
};

class HitscanRay : public RayObject
{
public:
HitscanRay(std::string EntityConfigFileName, std::string SceneName = "") : RayObject(EntityConfigFileName,SceneName);
void TestForCollision(void);//Test for collision takes the possibility of limited range into account. Once finished, ray will be marked as ready for deletion. At which point once its zombie time is expired, it will be deleted
protected:
unsigned int Range; //Tenths of a unit, so 10 = 1 unit.
private:
void InitialiseL2();
};

class ProjectileRay : public RayObject
{
public:
ProjectileRay(std::string EntityConfigFileName, std::string SceneName = "") : RayObject(EntityConfigFileName,SceneName);
void TestForCollision(void);//Once finished, checks the expire team, and if expired, deletes the ray -instantly-
protected:
unsigned int ExpireTime; //Time after which the ray expires. The weapon which fired the ray will store the -length of time- the ray is valid for (in milliseconds), when creating the ray, it looks this up, and adds it on to the current game time. After a check for collision, the time is checked: if the life has expired then the ray will be marked as such, and deleted at the end of the physics tick
unsigned int Speed; //Tenths of a unit per second, so 10 = 1 unit per second. Not affected physics update rate
private:
void InitialiseL2();
};

 

The pointer to base class makes it a breeze. Just tell each RayObject to TestForCollision and they will run their correctly defined version

 

classes form the majority of my project. I think more than structs and namespaces combined... It's just static classes that have always irked me. For me now, static classes as a means of access protection is no longer necessary.

 

I'm starting to get the idea that this secret namespace idea was much more common before Java came about than it is now. Sadly, it didn't translate directly into Java, whereas Java's workaround for it did translate directly back into C++, therefore you get more uniform code between the languages if you use the static class method. Of course I'm too young to know if that's actually true or not, but it sounds like it might hold water.

Link to comment

So the application is the owner of those pointers, I can live with that, unless you need to manage the lifetime. If this is not the case, it's a valid (but strange) implementation.

 

I do however remark some things in your code:

- Pass complex types as const reference: const std::string& EntityConfigFileName

- Don't forget your virtual destructors in enhetitence models.

- Add default constructors to your structs. Initialize the variables, always!

- Accessors should be const: bool IsExplosive() const;

Link to comment

- Accessors should be const: bool IsExplosive() const;

 

That's probably my java upbringing - we were never taught that at uni, so I just translated it directly. But now you mention it, it's so obvious... (If you're supposed to do that in Java too then I blame my uni lecturers)

 

Initialize the variables, always!

 

That's what the Initialise method does - all constructors call that. If there are derived classes, then L1, is for the base class's variables, L2 is for all which directly inherit from. L3 for all which inherit from that class etc (could cause confusion with multiple inheritance but right now, there is only single inheritance), since they can't have the same name without making it virtual and redefining.

 

 

You've linked to this post there and I've never heard of virtual destructors before.

 

- Pass complex types as const reference: const std::string& EntityConfigFileName

 

Is this because I'm not modifying the string (which I'm not, it's read only) or is it something else? I know const means (I think) 5 different things but the only one I ever remember is immutable variable...

 

unless you need to manage the lifetime

 

If I took the time to design it carefully I could probably make it so that they were only in scope when not on the front menu. However, I'm lazy and find it easier (in this case) to give them global lifetime, and simply reset the list when starting to load (and unload) a level. The list shouldn't get that big from a memory point of view, but will have entries rapidly appearing and deleting many times a second.

 

In a way I suppose I'm building it a bit like UT 99... You can be in the middle of a level, press escape and up comes your front menu (but the level is still there in the background - and in a multiplayer environment, very much alive and ticking). I'm basically building the same way, just starting with an empty level, rather than no level.

 

 

This is whay I always say, I might be able to write a program in C++, but no way am I a pro... I've still got a lot to learn

Link to comment

Const functions do not exist in Java, I won't blame your uni teachers :).

 

Regarding the variable initialization, you should do this:

struct CollisionHitResponse
{
       CollisionHitResponse()
       :
       NumEntitie****(0),
       EntityHit(nullptr),
       CollisionPoint(nullptr)
       { }

       unsigned int NumEntitie****;
       Entity * EntityHit;
       //unsigned int NumCollisionPoints; //A collision point is only saved if the entity the ray collided with is also saved, so these "count" variables will be the same, making one redundant - in this case, I chose this one.
       TVec3 * CollisionPoint;
};

You may not rely on the "fact" that another method "may" initialize the variables for you! Also a best practise, if I write "auto blah = new Whatever();" I expect that an initialized object. I often see creation followed by an initialization method, which is just plain evil because you can forget it. I create something that I want to use, so please create it completely.

 

Did you read the article? As you can see you must use virtual destructors or else the base object won't be destructed! So if you have inheritance, 2 things to do:

1) Check if the base has a virtual destructor, if not the object is probably not ment to inherit from (eg. std::string, std::vector, ...)

2) Set your own destructor virtual. This defines that your class may be inherited from or/and that you inherit from another class.

 

Why pass it as const reference is actually pretty simple. std::string allocates dynamic memory behind the screens, and if you pass it by copy you will get an extra memory allocation. There are 3 possible options.

void Foo(const std::string& bar) //You will only use bar, no changes allowed. This doesn't create a copy, but a reference to the same object! Performance is good.

void Foo(std::string& bar) //You will change bar, this is the original value that you pass to the function, be careful!!!!!!!!!

std::string blah("whatever");
Foo(blah); //OK
Foo("whatever"); //ERROR! You may not bind a temporary to a modifiable lvalue reference (ISO standard). But please note that Visual C++ allows this, also known as the evil extension...

void Foo(std::string blah) //Copy of original string passed to function, ideal if you will perform temporary changes on the string in your function.

 

No problems with sharing, this is the reason why I also like to share code, you learn a lot from other people! My remark to lifetime if probably because I coded for Nintendo DSi in the past. You really need all memory you can get, and having global instances is just not the way to do it. What we did were Singletons which we destroyed if unneeded: MyObject::DestroyInstance(); ftw!

Link to comment

Inititalizing ...

Do that in the constructors. Then if you feel better to do your resorce allocations etc etc.. in a method (there can be reasons for that) you should make it a virtual method.

class A
{
    char* _pmem;

public:
    A()
    :    _pmem(nullptr)
    {}

    virtual ~A()
    {
         DeInit();
    }

    virtual void Init()
    {
       DeInit();
        _pmem = new char[1024];
    }

   virtual void DeInit()
   {
        delete [] _pmem;
        _pmem = nullptr;
  }
};

class B : public A
{
    char* _pMoreMem;

public:
    B()
    :    _pMoreMem(nullptr)
    {}

    virtual ~B()
    {
         DeInit();
    }

    virtual void Init()
    {
        __super::Init(); 
       DeInit();
        _pMoreMem= new char[256];
    }

   virtual void DeInit()
   {
        __super::DeInit();
        delete [] _pMoreMem;
        __pMoreMem = nullptr;
  }
};

void somewhere()
{
      A a;
      B b;

     a.Init();   // A's init will be called
     b.Init();   // B's init will be called (which the internally calls A's init)

     a.Deinit(); // a's memory will be deleted
}
// end of scope. 
// a's memory is already deleted so nothing will happen 
// b's memory will be deleted including the part allocated by A
// If you had missed the virtual keyword for the destuctors, A's desctructor would never have been executed and you would have a memory leak

In case you allocate your class resources in a Init method I would recommend having a public virutal void DeInit() also and call that from the destructor.

 

About virtual destructors. In case you have any belief that your class will ever be inherited you MUST make the destructor virtual. And as its very hard to know what the needs are going to be in the future I would say. ALWAYS MAKE DESTRUCTORS VIRTUAL if you not has a good reason no to.

 

Keeping things in the constructors and destructors make things a bit more easy although it might not always be in line with you want.

 

class A
{
    char* _pmem;

public:
    A()
    {
        _pmem = new char[1024];
    }

    virtual ~A()
    {
             delete [] _pmem;
    }
};

class B : public A
{
    char* _pMoreMem;

public:
    B()
    {
           _pMoreMem= new char[256];
    }

    virtual ~B()
    {
        delete [] _pMoreMem;
    }
};

void somewhere()
{
      A a;   // a will be created and its memory allocated
      B b;  //  b will be created and its memory include the A-part will be allocated

}
// end of scope. 
// a's memory is deleted in its destructor
// b's memory is deleted in its destructor, The A-part destructor will also be called as its virtual

 

EDIT: Sorry Theo ... you wrote an answer while I was typing.. so I guess I just repeted some.

Link to comment

ALWAYS MAKE DESTRUCTORS VIRTUAL if you not has a good reason no to.

 

Sorry, but that's what you say to school developers in the first year C++ :). There is a huge performance impact as from you have 1 virtual function. It should actually be the other way around, If you inherit or want you class to be inherited from, make your destructor virtual...

 

A()
{
   _pmem = new char[1024];
}

A()
:
_pmem(new char[1024])
{ } //No default initialization. Modern compilers should optimize this...

Link to comment

Did you read the article?

 

I tried but the link pointed to my blog entry... A bit confusing since I've never heard of them before, but I think I'm starting to get the idea of what they are... I put "c++ virtual destructor" in Google and found this:

http://www.programmerinterview.com/index.php/c-cplusplus/virtual-destructors/

 

And I think Roland's answer says the same thing. So I can see why I might need it. Until I read that, I always thought that both constructors AND destructors were NEVER inherited from the base class because they had no name, but always had to be available if you wanted to use it. I think I've got a few classes to upgrade...

 

 

Regarding your improvement to the struct... I've never seen that before... That's effectively a constructor? Might make things a bit neater, but normally if I return a pointer from an function (or member function), it will be allocated and initialised as well, (unless I return 0 back, in which case no memory was allocated)...

 

But actually, I've noticed that the classes I put up earlier are in the middle of being changed. Notice the RayObject::CollisionHitResponse * CollisionHitData that has protected access? Notice how it has no accessor? "whoops" I was in the middle of re-designing it. Until then TestForCollision used to return a pointer (you had to declare it first, and then free it when you had finished), but I changed it to the pointer being a member instead.

 

The new idea is that it will automatically free during ~RayObject() which hasn't been written yet. The pointer is nulled during the Initialise method and as long as it stays null, it hasn't hit anything. If it hits something it allocates and initialises the struct - normally that would end the projectile's life (at the start of the next physics tick), but if not it can go back to a null pointer (after freeing it) once it's no longer colliding.

 

In fact, breaking news (international alert) I'm not even sure it needs to be a pointer... the NumEntitie**** field can act as the flag (zero = miss, non-zero = hit) - and if it's non-zero call realloc on the two pointers to make them large enough and then fills the data. So one less allocation when creating a RayObject (ie one less bit of cleaning up when finished :) )... Sounds like I've got quite a bit of festive coding to do...

Link to comment
ALWAYS MAKE DESTRUCTORS VIRTUAL if you not has a good reason no to.
Sorry, but that's what you say to school developers in the first year C++ :). There is a huge performance impact as from you have 1 virtual function. It should actually be the other way around, If you inherit or want you class to be inherited from, make your destructor virtual...
A(){    _pmem = new char[1024];}A():_pmem(new char[1024]){ } //No default initialization. Modern compilers should optimize this...

 

So you mean that I was wrong !!! How embarrasing :)

Well then there is only one thing to do - As you are completely right I have to go a kill my self :D

Link to comment

Here is the correction: REMEMBER TO MAKE YOUR CHILD CLASS DESTRUCTOR VIRTUAL

 

Not the parent class destructor? This is confusing... I thought virtual had no effect on classes that had no children...

 

 

 

Mind you, not as confusing as this: I went to click the reply button, but instead clicked report (I don't think anything happened, but sorry anyway) and I got this message:

 

[#10136] You do not have permission to report the item you are attempting to report.

 

 

So if there actually was something nasty, who DOES have permission to report it?

Link to comment

I removed the correction :)

Seems that I don't have to kill my self after all :)

 

-- case 1: No virtuals

#include <iostream>
using namespace std;

class Base
{
public:
   Base()	{ cout<<"Constructor: Base"<<endl;}
   ~Base()	{ cout<<"Destructor : Base"<<endl;}
};

class Derived: public Base
{
public:
   Derived()	{ cout<<"Constructor: Derived"<<endl;}
  ~Derived()	{ cout<<"Destructor : Derived"<<endl;}
};

void main()
{
  Base *Var = new Derived();
  delete Var;
}

 

Output

Constructor: Base

Constructor: Derived

Destructor : Base

Press any key to continue . . .

Only Base destructor called.... ERROR!!!

 

 

-- case 2: Base destructor virtual (this is what I said)

#include <iostream>
using namespace std;

class Base
{
public:
  Base()		{ cout<<"Constructor: Base"<<endl;}
  virtual ~Base()	{ cout<<"Destructor : Base"<<endl;}
};

class Derived: public Base
{
public:
   Derived()		{ cout<<"Constructor: Derived"<<endl;}
   ~Derived()		{ cout<<"Destructor : Derived"<<endl;}
};

void main()
{
  Base *Var = new Derived();
  delete Var;
}

 

Output

Constructor: Base

Constructor: Derived

Destructor : Derived

Destructor : Base

Press any key to continue . . .

All destructors called - OK

 

-- case 3: Derived destructor virtual

#include <iostream>
using namespace std;

class Base
{
public:
  Base()	{ cout<<"Constructor: Base"<<endl;}
  ~Base()	{ cout<<"Destructor : Base"<<endl;}
};

class Derived: public Base
{
public:
   Derived()	            { cout<<"Constructor: Derived"<<endl;}
   virtual ~Derived() { cout<<"Destructor : Derived"<<endl;}
};

void main()
{
   Base *Var = new Derived();
   delete Var;
}

 

Output

Constructor: Base

Constructor: Derived

Destructor : Base

Then ----> Crash : ERROR

 

So my statment seems valid. If you ever has a reason to believe that your class will be inherited you should have a virtual base class destructor. Maybe the school was right then :D

Link to comment

Oh ... and regarding the "[#10136] You do not have permission to report the item you are attempting to report."

I have seen that for weeks now. Nothing to bother about, the post works anyway.

Link to comment
Did you read the article?
I tried but the link pointed to my blog entry...

 

Strange, I may have made a mistake: http://www.codersource.net/c/c-miscellaneous/c-virtual-destructors.aspx

 

@Roland: I don't say that you are wrong, making all destructors virtual is just not the right solution. Your base destructor should ALWAYS be virtual, just like you stated. It is a good practise to also make your derived destructor virtual to state that you inherit from a base. If you don't want that your class is inherited from, you shouldn't do that however...

Maybe to remove the confusion, it's the same idea as:

 

class Base
{
public:
virtual ~Base();
virtual void Foo();
};

class Derived
{
virtual ~Derived();
virtual void Foo();
};

 

In Derived, you don't need the virtual keywords. But it shows that you overwrite a base implementation of Foo for example.

Link to comment

Haha .. Yeah Theo. We are apparently saying the same thing. But your are right of course. I those cases where you don't intend you make your class inheritable the virtual things are of course not needed at all. Would be nice to have a 'final' keyword in C++ :)

Link to comment
Guest
Add a comment...

×   Pasted as rich text.   Paste as plain text instead

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

×
×
  • Create New...