Introduce Node.visit(NodeVisitor)

[jheintz]

Why does this Visitor need to be internal? The Node.visit() method could be easily implemented externally and then no subclasses would need to implement the code.

I've checked in NodeUtil.visit() on dev_55 branch to demonstrate.

[MartinKersten]

Its following a pattern. Almost all AST / Node implementation I saw use the internal approach. A nice add is how people tend to learn api. In this way visit is offered by auto-completion.

Another thing is that you spotted potential DRY violation but your first idea is misleading. What you spotted was the need for AbstractNode base implementation. There may be other parts that will profit from a common base class. I dislike the Util solution. It reads better to use something like page.getRoot().visit(myVisitor) instead of Util.visit(page.getRoot(), visitor).

[jheintz]

I've actually already added AbstractNode in dev_3... it works pretty well without implication.

Let's drop NodeUtil.

On Wed, Oct 17, 2012 at 12:02 PM, Martin Kersten notifications@github.comwrote:

Its following a pattern. Almost all AST / Node implementation I saw use the internal approach. A nice add is how people tend to learn api. In this way visit is offered by auto-completion.

Another thing is that you spotted potential DRY violation but your first idea is misleading. What you spotted was the need for AbstractNode base implementation. There may be other parts that will profit from a common base class. I dislike the Util solution. It reads better to use something like page.getRoot().visit(myVisitor) instead of Util.visit(page.getRoot(), visitor).

— Reply to this email directly or view it on GitHubhttps://github.com/GistLabs/mechanize/issues/55#issuecomment-9535097.

John D. Heintz Agile, Lean, and everything in between

President, Gist Labs http://gistlabs.com Senior Consultant, Cutter Consortium http://cutter.com

Austin, TX (512) 633-1198

http://gistlabs.com