This post will be pre-writing for a LeoU issue. Recent discussions
<https://groups.google.com/d/msg/leo-editor/Nm0ZiVfd_ko/LlVXXXFCAQAJ> about
Leo's design have stimulated this post.
Here, I'd like to simulate a formal code review of Leo's vnode (VNode)
class. I'll wear three hats: the presenter, the secretary and the project
manager. I'll use Vitalije's comments as a starting point for a
discussion. This discussion would not normally be part of a code view, but
imo it's a good teachable moment.
See the summary for conclusions.
*Presentation*
The VNode class contains all data in an outline, as well as crucial,
low-level operations on those data. There is no other place to store
node-related data! In particular, the position class concerns itself only
with traversing the tree. The position class contains *no* node-related
data.
The VNode class is one of the foundations of Leo's scripting. As a result,
none of the ivars or methods of the VNode class can change in any way. That
is, all of the ivars of the VNode class are "official". Scripts can, and
do, access these ivars directly.
Theoretically, it would be possible to add new kwargs to VNode methods, but
that is extremely unlikely. It would also be possible to add new ivars to
the VNode class, but uA's make that unnecessary.
The properties of the VNode class, v.b, v.h, v.gnx, v.u, allow higher-level
access to various VNode ivars, but in some cases Leo's code accesses the
corresponding ivars directly, for greater performance.
*Discussion*
Here I'll respond to Vitalije's comments in detail. This would typically
not happen during a formal code review, but these comments will give me a
chance to more fully explain the design of the VNode class.
> one of the most basic element of Leo's data is certainly VNode. And yet
one can not instantiate VNode class without acquiring full blown commander,
which in VNode is known as context. In the comment of context attribute is
explicitly declared that it is called context to indicate its limited
usage.
Perhaps the "context" kwarg should just be called "c", but of course that
can't be done now. Almost all of Leo's classes have access to "c", the
present commander. There is nothing wrong with that.
What *data hiding* means to me is that the *boundary *between the "inside"
and the "outside" of a class is (relatively) strict. As far as the
"context" arg goes, the caller (creator) of VNode instances does not need
to know *anything *about how the VNode class uses the context.
The clients of the VNode class must not (and do not) know *anything *about
how the methods of the VNode class work. All methods (of all classes)
*must* be black boxes.
> However [the context ivar] is used not only as a holder of hiddenRootNode
but there is a method on VNode that knows about c.frame.body.wrapper and
knows that c.frame.body.wrapper has setInsertPoint, setYScrollPosition...
The VNode class must know everything about c.hiddenRootNode. It's another
VNode instance.
The vnode class must contain the v.scrollBarSpot ivar.
Yes, it would be possible to use c.restoreCursorAndScroll() instead of
v.restoreCursorAndScroll(). Ditto for c/v.saveCursorAndScroll(). But such
changes would have *no *consequences for Leo's architecture.
> [The VNode class] also knows indirectly about c.fileCommands.gnxDict.
To my knowledge, this is not true. The VNode class does not in any way
become entangled with other classes merely because v.context exists! The
VNode class knows *nothing* about how other classes use gnx's.
> Now, whichever module needs to access VNode class, can't just import
leo.core.leoNodes and use it. It needs to acquire commander instance before.
True, but so what? Every class in Leo has access to a commander instance
via its "c" ivar. That's why all Leonine scripts have the "c" var
predefined. Imo, it's useless to complain about this.
However, this comment does suggest creating c.new_vnode method:
import leo.core.leoNodes as leoNodes
# already exists in leoCommands.py
...
def new_vnode(self, gnx=None):
c = self
return leoNodes.VNode(context=c, gnx=gnx)
The (small) advantage is that clients of c.new_vnode(gnx) would no longer
need to import leo.core.leoNodes.
> Recently, we discussed the copying and pasting outline. It delegates its
task to fileCommands. FileCommands OTOH knows about
c.frame.resizePaneToRatio, c.selectPosition, c.frame.initialRatios,
c.atFileCommands.readAll, c.frame.body.onBodyChanged...
> It seems to me that copying and pasting outlines should belong just to
VNode. I would expect that v instance knows best how to encode itself into
a string, and to decode itself from string.
The VNode class does know about unicode. v._headString and v._bodyString
contain unicode characters. The v.b and v.h setter properties call
v.setBodyString() and v._setHeadString(), which handle the conversion to
unicode.
Ironically, the reason this is not obvious is that there is a strong
boundary between the VNode class and client code.
> There is no real need for v instance to know anything about
c.fileCommands, c.atFileCommands, c.frame.body.wrapper.
Nor does it, in any harmful way.
> You could say that fileCommands is designed to encode/decode vnodes.
> But why then it selects position, sets frame ratios
FileCommands class handles *everything *associated with reading .leo files.
That's its purpose. Yes, it must do a lot, but that can't be helped.
The FileCommands class creates VNode instances, without in any way knowing
the internal details of the VNode class. Yes, the FileCommands class must
know about gnx's, *but the converse is not true*. The VNode class knows
*nothing
*about how other classes use gnx's.
Crucially, the v.context ivar does not "pollute" the VNode class in any
way. The boundary between the VNode class and *all* other classes remains
intact.
> If fileCommands need to provide reading and saving Leo documents, then it
would probably use the same low level helper module as Vnodes for
encoding/decoding them from/to string.
v.b and v.h are properties that handle all the details of converting to
unicode.
The v.b and v.h properties are a perfect example of enforcing the
boundaries between the "inside" of the VNode class and everything else.
The boundary may not be apparent (because v.b and v.h look like ivars), but
the boundary exists nonetheless.
There is a huge practical advantage to using properties instead of
getter/setter methods on the VNode class.
Most Leonine scripters don't know what v.b and v.h actually are, which is
kinda the point. Otoh, those who want to understand the VNode class in full
detail *must *understand that v.context has no harmful architectural
consequences *whatsoever*.
> I could write many more examples but these few would suffice to make my
point clear.
I am glad to discuss any other Leo class.
*Summary*
The VNode class contains all node-related data. That's it's primary
purpose. It also contains crucial low-level operations on VNodes.
The ivars and methods VNode class must never change, except in an upward
compatible manner.
The VNode class is, in fact, an excellent example of information hiding.
The boundary between the VNode class is well defined and well enforced.
The v.context ivar is completely benign. It has never caused problems, and
it never will. It does not, in any significant way, make the VNode class
dependent on any other class. It has no harmful architectural consequences.
In my role as project manager, I pronounce the VNode class to be completely
sound, for the reasons just given.
See the Post Script for a summary of possible minor changes.
This thread is the place for all comments and debate about Leo's VNode
class.
Edward
P.S. Here is the "secretary's report", a summary of possible changes
related to Leo's VNode class:
1. A new c.new_vnode() method would slightly simplify the creation of new
VNodes by making it unnecessary for clients to import leo.core.leoNodes.
2. Leo could use c.restoreCursorAndScroll() instead of
v.restoreCursorAndScroll().
Ditto for c/v.saveCursorAndScroll().
Imo, having v.restoreCursorAndScroll() call a commander method is, for all
practical purposes, completely benign. This detail does not violate the
boundary between VNodes and the rest of Leo. In particular, code that uses
VNodes is completely unaware of this harmless "connection".
EKR
--
You received this message because you are subscribed to the Google Groups
"leo-editor" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
Visit this group at https://groups.google.com/group/leo-editor.
For more options, visit https://groups.google.com/d/optout.
