<div class="gmail_quote"><div> </div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
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<br>
</blockquote><div>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. This function is virtual only to give the derived classes the possibility to modify the state of some buttons in the toolbar, but it isn't required (in other words, it's a listener function).<br>
And Layout was still abstract -- in the sense that it can only be instantiated by a derived class, because its ctor and dtor are protected.<br><br></div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
Layout::ConfigurationChanged is not used at all either in Layout itself of in any of the classed derived from it<br></blockquote><div>Yes, I forgot to remove that. A working ConfigurationChanged() is in ModulWindowLayout::SyntaxChanged.<br>
<br></div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
similarly ExcuteGlobal & GetState I'd prefer if those were pure virtual, actually ExecuteGlobal isn't used in either class :-) so, it's not clear whether you actually intend to use them for something or not but it's confusing when you have like<br>
basides1.cxx:701<br>
if (pLayout)<br>
pLayout->ExecuteGlobal(rReq);<br>
which does nothing<br></blockquote><div>It did something, but then I refactored ObjectCatalog to BasicIDEShell, and ExecuteGlobal became empty.<br>But it may do something useful later, e.g. when we'll have buttons to toggle StackWindow and/or WatchWindow.<br>
</div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
virtual void UpdateDebug (bool bBasicStopped = false);<br>
<br>
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.<br>
</blockquote><div>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. This function allowed to remove GetStackWindow() and GetWatchWindow(), which both violate encapsulation and disallow BasicIDEShell to handle layouts generally.<br>
It's better not to be pure virtual since only ModulWindowLayout does something useful -- all other classes (currently there is only one other class) just do nothing, so an empty body is suitable for default implementation.<br>
<br>
Uray M. János<br></div></div><br>