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
Context
Privacy Policy |
Impressum (Legal Info) |
Copyright information: Unless otherwise specified, all text and images
on this website are licensed under the
Creative Commons Attribution-Share Alike 3.0 License.
This does not include the source code of LibreOffice, which is
licensed under the Mozilla Public License (
MPLv2).
"LibreOffice" and "The Document Foundation" are
registered trademarks of their corresponding registered owners or are
in actual use as trademarks in one or more countries. Their respective
logos and icons are also subject to international copyright laws. Use
thereof is explained in our
trademark policy.