Children() is hard to implement/overwrite

manticore-projects commented 2 years ago

Greetings!

First of all, thank you a lot for providing the software. It looks exactly like what I need. (For reference, please have a look at the AST visualization at http://217.160.215.75:8080/jsqlformatter/JSQLFormatter/demo.html -- I will want to do the same but in ASCII).

I have already a plain TreeNode structure (which I use for the Java JTree view) and find it now difficult to deal with your naming conventions:

1) TreeNode and hu.webarticum.treeprinter.TreeNode collide and make it cumbersome to work with both interfaces within one Method. Maybe a name change into AsciiTreeNode would make things easier.

2) content() is a matter of taste, maybe you want to direct it to toString() by default

3) most serious, children() is a challenge because there is no easy way to translate Enumeration<? extends TreeNode> children() into List<hu.webarticum.treeprinter.TreeNode> children() without traversing the whole tree and translate Node by Node.

When implementing both interfaces, children() collides.

So why can't we keep the Java Enumeration<? extends TreeNode> children() and add another method List<hu.webarticum.treeprinter.TreeNode> getChildrenList?

It would make life so much easier, when coming from JTrees and looking for an ASCII output only. Thanks for considering and all the best!

manticore-projects commented 2 years ago

This is how I have solved it:

  public static SimpleTreeNode translateNode(TreeNode node) {
    SimpleTreeNode simpleTreeNode = new SimpleTreeNode(node.toString());
    Enumeration<? extends TreeNode> children = node.children();
    while (children.hasMoreElements()) {
      simpleTreeNode.addChild( translateNode(children.nextElement()) );
    }

    return simpleTreeNode;
  }

  public static String formatToTree(String sqlStr, String... options) throws Exception {
    TreeNode rootTreeNode = null;
    // The Java TreeNode Structure
    JSQLFormatter.JavaObjectNode[] nodes = JSQLFormatter.getAstNodes(sqlStr)
                                                        .toArray(new JSQLFormatter.JavaObjectNode[0]);

    SimpleTreeNode rootNode = new SimpleTreeNode("Statement");
    for (JSQLFormatter.JavaObjectNode node : nodes) {
      rootNode.addChild( translateNode(node));
    }

    return new ListingTreePrinter().stringify(rootNode);
  }

For the example:

select 1
from dual;

It gives me this beautiful output:

Statement
 └─net.sf.jsqlparser.statement.select.Select
    └─net.sf.jsqlparser.statement.select.PlainSelect
       ├─<html><font color='gray'>selectItems -></font> Collection&lt;SelectExpressionItem&gt;</html>
       │  └─net.sf.jsqlparser.statement.select.SelectExpressionItem
       │     └─<html><font color='gray'>LongValue:</font> <em>1</em></html>
       ├─<html><font color='gray'>Table:</font> <em>dual</em></html>
       └─net.sf.jsqlparser.expression.operators.relational.EqualsTo
          ├─<html><font color='gray'>Column:</font> <em>a</em></html>
          └─<html><font color='gray'>Column:</font> <em>b</em></html>

davidsusu commented 2 years ago

Thank you for the feedback. Let me respond one by one.

Your suggestion is interesting, but the name AsciiTreeNode is not really good, since unicode is specifically supported, and ansi formatting is also planned to be supported in the near future. Anyway, I'll think about it.
toString() may seem very natural at first. My problem with it is that you can't add it to interfaces, and thus documenting it is also problematic.
First of all, Enumeration is a legacy interface. I might consider using the Iterator interface, but this freedom for TreeNode implementations places a heavy burden on TreePrinter implementations. Even if current implementations can be converted relatively painlessly. I'll think about it some more.

manticore-projects commented 2 years ago

It works so nicely, even with ANSI formatting. Thank you for this library, I love it.

davidsusu commented 2 years ago

I decided to reject these suggestions because of these considerations:

our TreeNode is not related to Swing's TreeNode in any way
usually, the former is useful when the latter is not even available
anyway, it's easy to make an adapter between the two types
TreeNode is a general interface, a more specific name would be misleading
the Enumerate interface is obsolete
children() should provide an ordered, countable and random-access collection (basically the definition of List)
content() is explicit, while TreeNode (being an interface) couldn't explicitly define the toString() method (but nothing prevents you from implementing it)

manticore-projects commented 2 years ago

Thank you for your time and consideration. Cheers!

davidsusu / tree-printer

Children() is hard to implement/overwrite #7