[antlr-interest] Complaints about BaseAST implementation

[Micheal J]

> Hi Andy
> 
> Having the fields is wrong due to a primary reason
> that  adding fields violates the design principle with
> which the class was created.

Don't think so but like Paul, I'd like to see your changes before making up
my mind.

>    BaseAST was clearly  created with the right
> decision to implement common algorithms.  Thats why
> its abstract. Algorithms, ideally, don't depend on how
> the data is stored and design is said to be solid if
> interface provides methods through which they can be 
> implemented. The methods are there in AST interface!

Perhaps BaseAST simply implements common algorithms for the CommonASTxxxx
classes. Anyone is free to design a new AST-Node class that implements the
behaviour specified in the AST interface.

> By putting fields, freedom is taken away from
> implementer how exactly the nodes will be connected to
> each other. eg. Some person may want to have constant
> time access to any given child of a node, which
> current  fields can't satisfy. So they are left unused
> (BAD!), when implementer adds the particular
> methods/fields.
> 
> I am against the idea that a class meant to hold
> algorithms should dictate in any way how the data is
> supposed to be stored.

One explanation is that BaseAST is a common subclass for a specific set of
concrete classes that store their data in the down/right fields.

> So, either all classes deriving from BaseAST should
> implement get/set child/sibling AND get/set text/type,
> OR a minimal concrete class can be provided that
> derives from BaseAST (other classes derive from this
> concrete class) and implements node connectivity
> (right/down) and returns empty string and 0
> respectively for getText() and getType(). Again,
> implementing them in BaseAST is incorrect. It is an
> abstract class and *must not* have any say in things
> it  has no info about.

Your insistence that it "*must not* have any say" might be mistaken for a
faith-based belief. The BaseAST class clearly has info about node
connectivity. You have an opinion about how it should [have] be[en] written
and that is fine but, it doesn't make all other opinions wrong.

Anyways, still looking forward to see your changes. Sounds like you've
managed to expand the set of classes that BaseAST can support.

Cheers,

Micheal