feat: introduce core TreeNodeVisitor pattern scaffolding (#262)#312
feat: introduce core TreeNodeVisitor pattern scaffolding (#262)#312dyrpsf wants to merge 3 commits into
Conversation
|
The mock visitor testing is officially in place! I added To resolve the cascading interface implementation requirements across the JSBML extensions, I added the standard |
draeger
left a comment
There was a problem hiding this comment.
It is a bit surprising to me that the inherited method is to be implemented in so many classes. JSBML has a very detailed and abstract type hierarchy. I am convinced it is possible to exploit this hierarchy better so that the interface method is implemented only in a very few top-level classes and can be removed from most more specific classes; in particular, all the specific ASTNode types should inherit the method from a more abstract class. Please check the users' guide again, which contains a class diagram, see https://github.com/sbmlteam/jsbml/raw/master/doc/user_guide/User_Guide.pdf
Also, I left a few comments suggesting that import statements could simplify the type declarations of arguments passed to a method.
|
Hi @draeger , thanks for the review! I have implemented all the requested changes:
The multi-module build is completely clean and passing. Let me know if everything looks good to merge! |
Description
This PR implements the foundational architecture for the
TreeNodeVisitorpattern discussed in #262. It provides a clean, generic way to traverse the JSBML AST without modifying core classes for every new feature.Key Changes
TreeNodeVisitor<T>: Created the core interface withvisit(SBase)as the primary method andvisit(TreeNodeWithChangeSupport)as the fallback for auxiliary nodes.TreeNodeWithChangeSupport: Added the<T> T accept(TreeNodeVisitor<T> visitor)contract to the root interface.defaultinterface methods), theacceptmethod implementation has been explicitly injected into the 18 required base classes (e.g.,AbstractSBase,ASTNode,XMLNode, and the various math nodes).All core tests are passing cleanly locally.
This PR serves purely as the structural scaffolding. Once this architecture is approved and merged, we can begin adapting tools like the
AntimonySerializerto actively utilize this visitor!