Re: cleanup patch / design rules

[David Philippi]

On Friday 23 August 2002 23:42, Ingo Ruhnke wrote:
> > Shallow pointer copies in objects are very dangerous and most often lead
> > to strange, hard to catch bugs. I'd guess that quite a few segfaults
> > were caused by this.
> Havn't had a segfault cased by that for years.

Well, we have quite a lot of segfaults that are hard to reproduce.

> We have quite a few classes that hold reference pointers to other
> objects, so just deleting them will not do any good. But we should
> probally name these pointers differently or make them via some kind of
> smartpointer undeletable or something like that.

Renaming them would be the least to be required. Sure, deep copy are not 
always what you want.

> If classes are small and only a helper class of another one, there is
> no need for an extra file, but the class should probally moved into
> the other class then.

I'd say that very small helper classes may become nested classes, other very 
small classes should end up having only a .hxx until they grow too much to 
have anything inlined.

> But please don't overuse namespaces, just add them where there really
> makes sense and not just but them everywhere just for the fun of it.

Well, inserting namespaces is a bit of work, I don't plan to do this 
everywhere. Namespaces are designed to partition your code and make it more 
understandable. I've already started to e.g. move every input button related 
class in it's own subdir. Given the fact that I wrote the hole code only 
recently I was surprised how much I had think before I knew what files I had 
to move and what not.
The thing I like most about those namespaces is that you get a list of all 
members of a namespace from Doxygen. Given enough, usefull applied 
namespaces this makes understanding the game much easier.

> The subdirectories Makefile.in needs to be added to the configure.in
> script and to the SUBDIRS variable of the parents directory. After set
> run ./autogen.sh.

Yeah, I found out with some trying myself. You forgot to mention that I've 
got to add the new .a to src/Makefile.am .

> In the constructor the action doesn't have a pointer to the Pingu
> which it might need for initalisation.

Without looking into the code - isn't it possible to create actions only with 
a Pingu pointer? What sense do those objects have unless they're coupled 
with a Pingu?

> > - May I make float delta a global constant of some sort?
> Only in the game-engines main-loop and everything that is called from
> there. Everything beside that needs its delta (GUI, Menu's, etc.). The
> game_speed variable already holds the needed value.

This means that everything called from World? down may use game_speed instead 
of delta, but the other classes have different delta's with the same name?

> Ok, even so I don't expect to ever have tons of docu.

Well, maybe that I'll write some. Maybe not. Anyway, there are already a few 
headers where there's so much docu that it hides the code.

> If the part code works for the player, thats enough. After all this is
> a game, not some contest to write the perfect C++ code ever.

After all, this game is intended to grow further. Growing clean code is much 
faster and less painfull then growing screwed up code. I certainly don't 
intend to make the code perfect, but I intend to get it to the point where 
anyone with good C++ knowledge may look at it and understand what's going on 
without too much unneccessary trouble.

> #ifdef HAVE_SSTREAM
>   std::ostringstream oss;
> #else
>   std::ostrstream oss;
> #endif
>   oss << any;
>   return oss.str();
>   oss << any << std::ends;
>   oss.str();

> The code under the return looks a bit unreachable.

Hmm... I guess you're right. ;-)
Must be related to the fact that the #ifdef was larger before. Seems that I 
overlooked the return when chaning this.

Bye David