On Thu, Jan 16, 2014 at 10:13 PM, Edward K. Ream <[email protected]>wrote:
The read code consists of three *completely independent* phases:
>
> 1. Initing the OrganizerData nodes and other global data
> (precompute_all_data & helpers)
> 2. Generating the global moved node list (demote_helper & helpers)
> 3. Actually moving the nodes as specified by the list (move_nodes &
> helpers)
>
I am going to completely rewrite phase 2. It's starting to smell bad.
The fundamental problem is that the code must distinguish between regular
nodes, imported organizer nodes and existing organizer nodes, and possibly
even variants of the above. This is a hopeless strategy: every combination
of the various kinds of parent/child nodes looks like being important.
A much simpler strategy is possible, one that treats all nodes exactly the
same!
The existing code calls demote_helper for each organizer node (but not each
existing organizer or imported organizer node). Instead, the new code will
traverse the imported outline just once, visiting each imported node
exactly once. Here is how it will work:
1. The new main line, say vc.demote, will visit each node of the imported
outline, in outline order.
2. At each position p of the traversal, vc.effective_unl(p) will calculate
the **effective unl** of p, taking into account all organizer nodes that
have been *simulated* to be in the outline at any particular point.
3. Now we ask, is there a match between vc.effective_unl(p) and any of the
*unaltered* unls in *all* the @organizers: nodes?
It will be trivial to make this test: just create vc.unl_dict, whose keys
are (unaltered!) unls and whose values are OrganizerData (od) nodes
corresponding to the @organizer: and @existing-organizer: nodes in the
@views tree.
If there is a match, giving od1, we close out any od nodes that are not
parents of od1, enter od1, make p part of od by adding p to the global move
list. Otherwise, we add p to the pending list. This is pretty much
exactly the way that the old demote_helper works!
Some of the details are a fuzzy, but the important features are:
1. It doesn't matter what kinds of nodes contribute to vc.effective_unl(p)!
2. There are no special cases involving unls: we simply look up
vc.effective_unl(p) in vc.unl_dict!
3. vc.demote will visit each imported node exactly once. This eliminates
worries that contributed to the downfall of the old code.
===== Summary
The new phase 2 code will treat all (parent) nodes in the same way. It has
been gradually dawning on me that trying to distinguish between various
kinds of organizer nodes is a recipe for lots and lots of bugs.
The new code will eliminate several complications in phase 1. Phase 3 will
largely remain unchanged (phase 3 still needs work).
The new demote method will be a relatively easy refactoring of the old
demote_helper method and its helpers. The only truly new code is the
node-by-node computation of the effective unl, depending on a stack of
simulated (and real) parents.
In short, the new phase 2 promises to be fundamentally simpler and more
reliable than the old way.
Edward
P.S. Let me attempt to explain in more detail why the old way suddenly
collapsed. The last straw was this code in demote_helper:
if od.exists:
iter_,tag = od.parent.following_siblings,'sib'
else:
iter_,tag = od.parent.children,'child'
In other words, if od is an existing organizer, we have to look a sibling
nodes, while in all other cases we have to look at child nodes. This
distinction leads to other "if" statements in the main loop.
Game over: the number of paths through the code is (in some vague, but real
sense) the number of the "if" statements *multiplied* by all the ancestors
and/or descendants of the od node. Unit tests (and the code itself!) would
have worry about all the tests.
You can see now why the new code is so much simpler than the old. All
nodes are traversed in *exactly* the same way. For example, it doesn't
matter that a parent organizer node happens to be an existing organizer
node. The existing organizer will contribute its headline to the effective
unl just like all other parent nodes (of *whatever* kind), so p will match
the unl in the @existing-organizer: node even though p is *actually* (in
the not-yet-changed imported outline) a sibling of the existing organizer
node. Do you see how clever this is?
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 http://groups.google.com/group/leo-editor.
For more options, visit https://groups.google.com/groups/opt_out.
