Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extend TreeNodeWithChangeSupport with Visitor pattern #262

Open
Schmoho opened this issue Aug 12, 2024 · 1 comment
Open

Extend TreeNodeWithChangeSupport with Visitor pattern #262

Schmoho opened this issue Aug 12, 2024 · 1 comment

Comments

@Schmoho
Copy link

Schmoho commented Aug 12, 2024

This is a suggestion: It seems to me that the current implementation does not support the idiomatic way to perform tree walks in Java, because the TreeNode interface that is used does not support the Visitor pattern.

That is, doing a tree walk now requires to explicitly handle type dispatch in the walking class, which is kind of painful to do when also accounting for Plugins and Version differences.

I am very uncertain whether my current understanding of JSBML is correct in that regard and also I am kind of shaky on Java patterns, so please feel free to correct me.

Since the TreeNodeWithChangeSupport interface is the central interface which allows to perform generic tree walks on SBMLDocuments, I would suggest to add at least one interfacesMutatingTreeNodeVisitor with signatures public void visit (Species s) etc. TreeNodeWithChangeSupport would then have void accept(MutatingTreeNodeVisitor v).

@draeger
Copy link
Member

draeger commented Aug 26, 2024

Thank you for this interesting suggestion. In my point of view, the TreeNode interface that is on the top of JSBML's type hierarchy was placed in the wrong Java package. There should have been a general algorithm or patterns package but instead this important interface is part of the SWING graphical user interface package. Your idea of making the derived interface TreeNodeWithChangeSupport more compatible with recent developments seems sound. However, modifications to a top-level generic interface may have many (potentially also unwanted) effects. In addition, an implementation needs to provided in one of the implementing abstract classes (or a default implementation if suitable). Could you elaborate what subsequent changes to the type hierarchy or implementing classes would be needed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants