[antlr-interest] Complaints about BaseAST implementation
atripp at jazillian.com
Wed Oct 12 14:20:59 PDT 2005
OK, I see what you're saying now. Split BaseAST into two: a base class
that has no reference to "down" and "right" fields, and a subclass that
I guess that makes sense. A couple of things though:
* the subclass will need to be called BaseAST and not the base class,
or else everyone's code will break.
* You'll still want the "type" and "text" fields in the base class, I
I can't imagine an AST that doesn't need these
* I'm still having trouble picturing why you would want to implement
these "down" and "right" fields any other way. I don't doubt that you need
to do it, I'm just saying I can't picture it. So in the end, it may not
it for Terrence to fold in this change, because you may be the only
person who ends up needing/wanting it. I know there's no real harm in
him doing it, but it does add a little complexity.
And in defense of BaseAST (and it's author :), it's certainly not
someone might not want to keep "down" and "right" fields, and in fact I
that you may be the only user who wants/needs to. So I think the
"this BaseAST design is awful" is unwarranted. Making your change may
be an improvement, but the current design has worked just fine for every
for many years, AFAICT.
Akhilesh Mritunjai wrote:
>--- Andy Tripp <atripp at jazillian.com> wrote:
>>All ranting aside, I don't see how BaseAST can
>>and thus provide methods like getFirstChild() and
>>having any fields. Can you say specifically how that
>>can be done?
>It doesn't have to! Thats what my point is.
>BaseAST is abstract, so there is no obligation on it
>to implement anything it has no idea about. It is the
>duty of concrete class inheriting from it to provide
>concrete implementations of set/getFirstChild() and
>set/getFirstSibling() etc. How the concrete class
>implements them is again of no concern of BaseAST. The
>contract from AST interface says they return an
>instance of type AST... so as long as it is fulfilled
>by concrete class everything should be fine.
>>But BaseAST itself implements the tree
>Thats what is wrong. BaseAST should not do it. It
>implements just the algorithms, which should have
>nothing to do with how data is *actually* stored.
>>If you want to do that yourself, you
>>be subclassing BaseAST.
>But there is problem with it. BaseAST implements a ton
>of algorithms which work just fine. They are tested
>and would work (may not be most optimally) whatever
>way data is stored. I don't want to reimplement (and
>debug) them again... I don't even want to copy/paste
>them in MyBaseAST (which will be *identical* to
>BaseAST with the two fields less).
>So eg, addChild() would work fine even if childs are
>stored in a doubly linked list, however, if the
>implementer wants better performance she/he might
>choose to implement it in fashion that gives O(1)
>performance rather than O(n) which is right now.
>Again, it is implementer's choice.
>Yahoo! Music Unlimited
>Access over 1 million songs. Try it free.
More information about the antlr-interest