Re: [Pingus-CVS] CVS: Games/Pingus/src surface_button.cxx,1.12,1.13

[David Philippi]

On Monday 09 September 2002 12:50, Ingo Ruhnke wrote:

> If you want to do it I can give you the specs.

Maybe that splitting the remaining WorldObjs takes a bit longer than I 
thought (at leat Teleporter was an ugly mess) but you may well give them to 
me. I guess it's more interesting code than splitting very old classes. :-)

> Forget that, the current structure is fine and WorldObj has exactly
> the same things in common with WorldObjData that EditorObj has.
> WorldObjs::Data:: would be incorrect and would not represent the
> structure. Beside that I like mostly flat directory structures.

Yes, I've learned a bit more about EditorObjs while working on Teleporter. 
Moving worldobjsdata inside worldobjs wouldt be wrong. The problem here is 
more that the name is a bit misleading at first. One tends to forget that the 
EditorObjs are as deeply interwined with them as WorldObjs.

> BTW. I would like to get rid of all those new and deletes:
> I don't like them and they don't provide any real benefit. Having real
> objects instead of pointers would look much nicer. And beside that I

Maybe it'd look nicer, but it would force me to include every header again 
leading to the situation where one change to e.g. the documentation of 
WorldObjData.hxx yields a recompile of most files. I dimly remember that 
there's some option to make to prevent this, but this isn't supported by my 
IDE and it's unlikely that I'd remember to do the compile by hand. Then 
there's the problem that it does not only increase compile time but also the 
memory usage while compiling and the size of a compiled source tree.

> think the private inheritage looked quite a bit nicer, since it
> avoided all those data->blabla...

Does it? It's shorter for sure but abstraction isn't always what you want. 
It's the same as with C vs. C++ cast. A C cast seems to be superior - only 
one cast for everything and short to type... well ok, dynamic_cast<> is 
usefull since it returns 0 pointers / throws exceptions. But we don't need 
the other ones? Wrong. Both serve one important function: they are 
obvisious. You can't miss them in the code very well. Using private 
inheritance would make the boundary between code and data invisible again. 
If the change had only been made to remove the inter header dependencies I 
could easily have added things like:
.hxx
CL_Vector& pos

.cxx
  pos(data->pos)

and used a simple pos. While I also don't like the data-> all too much 
(especially while adding it everywhere) I'm well aware that it makes it more 
easy for other programmers to understand how the code works. I can assure 
you that it's not easy to understand the old structure and I'm not sure if 
I'd ever have taken the time to do it without the possibility to ask you 
about some points instead of digging them out of the code.

Moving to private inheritance again would be even worse since we would then 
again have three large interwined class hierarchies. I can't tell you from 
my own experience that such a thing is very daunting and hard to comprehend. 
One should use private inheritance only when layering doesn't work due to 
the fact that e.g. protected data members are required or perhaps pure 
virtual functions.
My IDE has functions to show me which member is from which base class, wheter 
a method is overwriten etc. pp. But honestly, I use them only if I can't 
understand the code without and thereby I'm using it seldom enough that I 
still don't know all the symbols used. I prefer to stay in source view very 
much.

If you just want to get rid of the new's and deletes we may change the data* 
to auto_ptr<data*> and the create_WorldObj() from new WorldObj(this) to new 
WorldObj(new Data(*this)).

I don't consider getting rid of the new/delete very usefull and getting rid 
of the data-> harmfull, even more so if it's done by switching to private 
inheritance again instead of simply using references as alias to the full 
name (may it be data->X or data.X) .

If you remember - I started working on this code with the goal to remove 
those double inheritance.

Bye David