[
https://issues.apache.org/jira/browse/DERBY-673?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13718153#comment-13718153
]
Knut Anders Hatlen commented on DERBY-673:
------------------------------------------
Maybe something like this would work, and make it clearer which nodes need to
implement the isSameNodeKind() method:
1) Keep ValueNode.isSameNodeType() as it is today (it seems like it's still
useful as a shorthand, as the isEquivalent() methods in the patch perform the
same check manually)
2) Make UnaryOperatorNode.isEquivalent() and BinaryOperatorNode.isEquivalent()
check other.kind == this.kind directly, since they cast the received object to
the specific type anyway
3) Introduce an isSameNodeKind() method in ConstantNode (and sub-classes as
necessary) for use in ConstantNode.isEquivalent()
I think that if isSameNodeKind() is defined in ValueNode, it is more likely
that it will be used as a general utility method, so it should be overridden in
all nodes that use kinds, for consistency. But it sounds unnecessary to
implement this method in classes where we don't expect it to be called, so I'd
prefer that we keep it local to the sub-tree (ConstantNode) that seems to need
the ability to override the behaviour.
If we give the method a prominent place, such as ValueNode or its own NodeKinds
interface, I think we should give it better defined semantics. Currently, the
base method has two possible outcomes (true or false), whereas the overrides
have four (true, false, NullPointerException or ClassCastException).
> Get rid of the NodeFactory
> --------------------------
>
> Key: DERBY-673
> URL: https://issues.apache.org/jira/browse/DERBY-673
> Project: Derby
> Issue Type: Improvement
> Components: SQL
> Reporter: Rick Hillegas
> Assignee: Dag H. Wanvik
> Labels: derby_triage10_11
> Attachments: derby-673-1.diff.gz, derby-673-1.status,
> derby-673-2.diff.gz, derby-673-2.status, derby-673-3.diff.gz,
> derby-673-3.status, derby-673-fixcomments.diff,
> derby-673-more-typesafe-6.diff, derby-673-more-typesafe-6.status,
> derby-673-nuke-ctypes-enum.diff, derby-673-nuke-ctypes-enum.stat,
> derby-673-nuke-ctypes-without-enum-2.diff,
> derby-673-nuke-ctypes-without-enum-2.status,
> derby-673-nuke-ctypes-without-enum.diff,
> derby-673-nuke-ctypes-without-enum.status, derby-673-typesafe-lists-1.diff,
> derby-673-typesafe-lists-1.status, derby-673-typesafe-lists-2.diff.gz,
> derby-673-typesafe-lists-2.status, nodefactory-31.status, nodefactory-31.zip
>
>
> This piece of code once had a purpose in life. It was one of the
> double-joints which allowed cloudscape to ship with and without compiler
> support for the synchronization language. Synchronization has been removed.
> If we want to plug in optional language components, I think there are better
> ways to do this.
> The NodeFactory turned into a big, sprawling piece of code. At some point
> this code was slimmed down by telescoping all of its factory methods into a
> couple unwieldly, weakly-typed overloads backed by cumbersome logic in the
> actual node constructors. I would like to reintroduce strongly typed node
> constructors which the parser can call directly. This will make node
> generation easier to read and less brittle and it will get rid of the now
> useless NodeFactory class.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
