Re: cleanup patch / design rules

[David Philippi]

On Sunday 25 August 2002 16:22, Ingo Ruhnke wrote:

> Most (or at least some) are parent-pointers. GUI elements have a
> pointer to the GUIManager and such.

Those may safely be replaced by references. The parent should exist longer 
then the child in such cases. References are even a little bit faster then 
pointers since they don't have to be dereferenced.

> When the pointers are just there for caching purpose, they should
> probally be removed and replaced with an inline getter method. If
> calling the getter-method looks to ugly, like:

Don't forget that virtual methods can never be inline.

> one should add the caching varibable to the member function that uses
> it and not to the class.

Yes, caching in the member function is probably best in many cases.

> That might work in a few cases, but in general references are to
> inflexible (no null element, no way to reset them, etc.), so I avoid
> them mostly in classes.

That a reference always points to something is considerd to be one of it's 
greatest advantages. They require a different programming style then many 
people are used from C but if you use them right you code will be much less 
error prone.
I did never write any significant amount of C code, so I never got used to 
using pointers so much. When I code you'll see very little pointers (if 
any), but many references. As long as we don't want to rewrite large parts 
of Pingus it may be neccessary to use some dummy objects as initializers / 0 
replacement for caching references, but caching pointers which are not 
always initialized should be moved into the methods anyway IMHO. I suppose 
that there are quite a lot of cases where it's more appropriate to return a 
[const] reference instead of a pointer, in fact every get_X* where the 
caller isn't supposed to delete the pointer will probably be better of with 
a reference. If you catch those in a reference using an inline get method it 
will be a no op.

> We should probally add some wrappers around new/delete to make it
> easier to track them and to add some bookkeeping to catch double
> deletes, etc.

What new and delete do you mean? The global ones? operator new/delete? The 
array variants? There's a bit more to cover than one may think at first 
glance. I consider changing the behaviour of new and delete very dangerous 
since one doesn't expect them to be overloaded.

Bye David