I have found some further issues regarding toString, treePrint and
printSubNodes. The comments only apply to subclasses of
- Some class variables' value are printed with a leading "<label>: "
in front of the value, others do not have a label, the value is
printed alone. Usually, the label is the same as the class
variable's name. Shouldn't this always be the case for uniformity?
- Some class variable labels are printed even if the values is not,
in cases where the variables value is null, e.g. "preJoinFL: " in
SelectNode. I suggest not printing in such cases to save space.
- Some toString methods call super.toString before formatting the
class's own member variables, others do it the other way around. Is
there a rationale for this I wonder? Or is it just accidental? It
seems the established pattern is that super be called after, except
for DDL operations, which always call super first, so I suggest
adjusting toString to those two patterns throughout.
Non-DDL classes that call super.toString first:
- Some usages of toString terminate with a newline, most do not. I
think they should either end with a newline (DDL) or newline + call
to super.toString (DML). At the root (qua anchor),
QueryTreeNode.toString just prints "", so effectively for DML, the
last thing printed will be the newline also.
- I think all nodes should call super.toString. This could be because
the author "knew" they are right under QueryTreeNode, and so has
chosen to optimize the call away (QueryTreeNode.toString prints the
empty string), or it could be an omission. I think it would be good
to be consistent here, too.
Example of nodes that do not call super.toString at all:
TableName is possibly an allowable exception, I think, because its
toString will return a string even in insane builds, so I not sure
it can tolerate a terminating newline, so I intend to not touch
- Some nodes call toString of underlying nodes in their own toString,
but such subnodes should, according to the contract defined in
QueryTreeNode, be handled by PrintSubNodes/TreePrint:
DistinctNode (formats childResult)
GroupByNode (formats childResult)
OrderByNode (formats childResult)
OrderByColumn (formats expression)
I suggest bringing such instances into line.
- Vectors are handled specially. Vectors with tree elements are
subclasses of QueryTreeNodeVector, which contains a generic toString
method which calls toString of the elements of the vector and
concatenates them. I suggest changing this to be printSubNodes and
use series of calls to treePrint instead to comply with the general
pattern. Subclasses which have no additional members need not
override toString either.
- Some subclasses of QueryTreeNodeVector needlessly define their own
printSubNodes method. I propose to remove those:
this now inherit QueryTreeNodeVector's printSubNodes.
- One subclasses of QueryTreeNodeVector (TableElementList) rolls its
own toString to print the elements. I suggest to remove this an
dinstead rely on the general mechanism outlined above.
Some classes neglect to print own member variables:
- Some subclasses of QueryTreeNodeVector define their own treePrint
This seems to be a remnant from a time when ResultColumnList was not
a subclass of QueryTreeNode, c.f this comment in
- This class needs a treePrint method, even though it is not a
- descendant of QueryTreeNode, because its members contain tree..
so I intend to remove this.