John Levon wrote: >On Fri, Dec 14, 2001 at 12:48:23PM +1100, Ben Stanley wrote: > >>>what's the point in calling back like this ? This looks like it requires >>>an VisitInsetX for every class X ? And furthermore, one that must be >>>public. >>> >>The point of calling back is that it gives you the virtual dispatch. >>It's the whole point of the scheme. Yes, it does require a VisitInsetX >>for every class X, but if you inherit from an InsetVisitorNOP class >>which provides default implementations for VisitInsetX for every class X >>which are empty, then this point is moot. >> > >this also applies to an iterator scheme with allowSpellCheck(). > *** With the visitor scheme, the spell check method would actually be done in the visitor, not the Inset.
This removes all spell check code from the inset. *** All the inset needs to provide is a way of accessing it's characters. This also facilitates other external methods anyway. If, for some strange reason, you had to build a LyX without spellchecking code, it would be easy - just don't include the spell check visitor file in the compilation, and it's gone. So this scheme allows you to easily add/remove features without changing the inset code (once you've added Accept methods to every inset). This allows for easier addition of new features without touching/breaking unrelated code. >>>OK, ugly, but we are doing RTTI here. Having inner spell code public (or friended, >>>which is not much nicer) is also ugly ... >>> I'm saying the spell check code doesn't belong in the inset, it belongs in the visitor. That way, all the different ways of spell checking insets are in the same place (if any insets need to have it done differently). >>It also means that every time you want to add a new fandangled operation >>on an inset, you have to >>a) modify Inset to have an allowX() method >>b) modify the class(es) of interest to support the new fandangled operation. >> >>This way, all the mods are done in one place, without ever touching the >>Inset classes (apart from adding the visitor support infrastructure in >>the first place). >> > >iterator way: for new operations, add a new Accept() (or similar) for each inset > that wants it (also use virtual where possible to avoid this) > and modify the base class, and have to re-compile the whole inset tree, and anything else that uses it. >your way: for new operations, add a new Accept() for inset that needs it (also use > virtual where possible to avoid this) PLUS a new thing in the calling class for > every type > No, Add Accept once only per inset. Subsequently only add a method to your visitor class. Only re-compile the visitor class - test quickly.
