kicad-developers team mailing list archive

[Lorenzo Marcantonio <l.marcantonio@xxxxxxxxxxxx>]

On Fri, Jun 06, 2014 at 08:30:59PM +0200, Tomasz Wlostowski wrote:

> Either use only C++ RTTI or a type ID field. Mixing both is ugly.

Or better yet trust your interfaces:P if you need to know the kind of
object you have, then there is a design issue, usually. Also type ID
doesn't handle inheritance easily, which is bad.

> This means a pretty much complete rewrite of Kicad...

Of course. That was meant for *new* code.

> I was reading Clang/LLVM source code and found this dyn_cast/isa pattern
> very elegant and lightweight. It's a mine of practical OO design ideas.

I could argue about the elegance of the whole template cruft:P it seems
that in 'modern' C++ you need to write more boilerplate/magical
definition than actual 'work' code.

> - it's much faster than C++ dynamic_cast,

Same analysis I did.

> - you don't have to remember the type ID <> class association (e.g.
> PCB_LINE_T vs class DRAWSEGMENT)

Have you looked for using typeid? could be an interesting alternative
(altough I don't seen any obvious advantage over the kicad type tag,
other than being a standard construct)

> - can be implemented without disrupting the code

Other than all these ugly things around :D

> - saves a line (or three lines) of code on every down-cast:

Agree with that. Still I found that usually you need to check the type
before the downcast anyway, so you have to do the if statement... also
the downcasted pointer has reduced scope which is IMHO a big advantage.

The common pattern I've found is:

if (stuff->type is CLASS_T) {
    CLASS *ptr = static_cast<CLASS*>(stuff);
    // do things with ptr
}

with your solution the pattern would be:

CLASS *ptr = dyn_cast<CLASS>(stuff); // More or less
if (ptr) {
    // do thing with ptr
}

When you expand the template the resulting code is pretty much the same:

CLASS *ptr = (stuff->type is CLASS_T) ? static_cast<CLASS*>(stuff) : NULL;
if (ptr) {
    // do thing with ptr
}

In the end it's down to individual preference, almost like indentation
conventions...

For the other people however I'd add a big comment explaining how it
works and the limitations (i.e. you need to define the correct ClassOf
statics and maybe other things). I guess it *could* handle inheritance,
putting the closure of the derived classes in the parent (so that an
hypotetical EDA_ITEM::ClassOf would contain almost all of the class
tags:P)

I've had bad experiences wrestling with the TRACK/VIA/SEGZONE class
branch virtuals...

-- 
Lorenzo Marcantonio
Logos Srl