Add #lazy_visit to Rubinius::AST::node #1636

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
3 participants
Member

txus commented Mar 26, 2012

The #lazy_visit method accepts a visitor but it does not automatically iterate over child nodes. From the method docs:

Just like the #visit method does, #lazy_visit calls a method
(determined by the #node_name method) on the visitor. It differs from

visit in the sense that it does not traverse the tree, leaving this

up to the Visitor.

This satisfies some Visitor pattern use cases in which the visitor
needs to decide when (or whether) to iterate over a particular node's
children, e.g. when implementing a source-to-source compiler.

Do you think this has its place in Rubinius::AST::Node, or is it too specific of a case? (Also, the naming could be better).

Add #lazy_visit to Rubinius::AST::node.
The #lazy_visit method accepts a visitor but it does not automatically iterate
over child nodes.
Owner

brixen commented Mar 26, 2012

A visitor pattern already breaks some encapsulation but the trade-off for the coupling is acceptable because of the generality of the mechanism.

I did not add a method like this because I consider the decision to process a node's children to be a component of the specific visitor's state.

I would like to see what the code looks like using these approaches before deciding whether to include this method:

  1. The visitor maintaining state that says whether to process node X when it is a sub-node of Y
  2. The visitor deciding to call #lazy_visit (or whatever the method is named) when node X is a sub-node of Y
  3. Changing #visit to required a return value that specifies whether to recurse into the children when the node is Y

Do you have time to code these three approaches so we can compare them?

I would be most inclined to support 3 above. It is still generic, without duplicating the method and allows for a mixed approach on any particular iteration of a tree, as opposed to 2 above which requires deciding to use #lazy_visit in place of #visit.

To be clear, both 1 and 3 require maintaining some state in the visitor, but the logic associated with that state appears preferable to logic deciding between calling #visit or #lazy_visit.

Member

txus commented Mar 29, 2012

Ok, I'll update this as soon as I can :) Thanks for all the feedback.

This pull request passes (merged 434bdcb into b4ab27c).

Member

txus commented Sep 30, 2012

@brixen I'm not sure why but I don't see the need for this anymore. I just coded a custom visitor for a sample source to source compiler, to see where I needed lazy_visit, and I didn't. Somehow it feels easy and understandable to me now. But if you think other people could benefit from a special interface being baked into AST::Node, let me know. If not I'll just close this. :)

Owner

brixen commented Oct 1, 2012

@txus I definitely think managing the state in the Visitor instance is preferable. As in my original note, we could add a return value flag if you find this is really necessary.

I'll close this for now.

@brixen brixen closed this Oct 1, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment