[PUSHED] [PATCH] Object Catalog in Dialog Editor

[Noel Power]

Bah, I meant to reply to this but got hung up with something else and forgot
On 21/08/12 17:33, János Uray wrote:
>
>     the Layout class has some virtual methods with no implementation,
>     this sounds like these need to be pure virtual making Layout an
>     abstract class ( which makes sense ) When I looked a little close
>     at some of those methods I notice
>
> IMO, not all empty virtual functions need to be pure virtual. Pure 
> virtual functions are needed when the derived class really need to do 
> something (e.g. returning a value). But GetState() is not that case.
My point is not whether an empty function needs to be pure virtual or 
not, my point is that an empty stub function buried in the base class 
implementation is not helpful when reading the code. For ease of 
readability the more information in the header ( without the need to 
look in the implementation the better). In this case there are only 2 ( 
and more than likely only ever 2 ) classes derived from the base class, 
the pure virtual ensures that one knows exactly where to look for the 
implementation ( in the specific derived class ), alternatively an 
inline implementation in the header would be fine too.
>
>     virtuals with defaults are not really recommended, I removed the
>     default, actually for this method, would be greate if we could
>     remove it from this base class altogether, it doesn't really fit
>     since Dialog has no concept of the debugger.  The Layout class is
>     a container and that method depends on assumptions about the
>     contents of the container. Anyway I didn't see an easy way out of
>     removing it at this point, probably something to keep in mind to
>     try and remove from here in the future, it wont do any harm to
>     leave it there.
>
> It's name is UpdateDebug() because it updates the StackWindow and 
> WatchWindow -- both are needed to debug Basic. I'm not very good at 
> inventing names.
I don't have a problem with the name, I just don't believe this method 
belongs in the Layout class, I don't buy that DialogWindowLayout will 
ever be interested in knowing if basic is stopped or not.

Anyway I am no purest, I wouldn't get hung up on those details, they are 
just things to keep in mind

Noel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/libreoffice/attachments/20120824/1b5cfcb4/attachment-0001.html>