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

Implementing a tree node interface #5227

Closed
wants to merge 3 commits into from

Conversation

Hackwar
Copy link
Member

@Hackwar Hackwar commented Nov 27, 2014

Goal

Currently we have a few constructs in our system that build a tree, but no proper interface for that. For example all menu items are forming a tree with nodes having a parent and children. Likewise with the categories and usergroups.
In the future, this interface should be implemented in those cases in order to provide developers with something to build on. For the moment, this would only be implemented in JCategoriesNode, further implementations would have to wait until Joomla 4.0, since they would break B/C.

History

This PR is a result of the router changes that are currently being implemented. The idea is, that a generic router rule can work with data obtained by a specific component from its router and with this interface know what to do with it.

Problems

My main issue right now is, if I have all bases covered or if I am forcing already too much unto future developers. I took JCategoriesNode as an example and removed what was category specific. I also removed the getSibbling() method, since I fear it is too much for a generic node interface/class. In JCategoriesNode this method is meant to allow navigation between sibbling categories and having simple arrows left and right on a category page to navigate between them.
While this interface implements all necessary methods for navigating between nodes, it does not provide means to enforce specific members of the class. From my POV we would need at least the class members $id, $title and $alias, but I would hate to create getters and setters for those simply because PHP does not support members in interfaces. So should this maybe even be an abstract class? That again would make this whole thing more difficult to adapt for third parties... Thoughts?

Current Status

This PR is NOT ready for merging. It is meant for a place to discuss this idea and improve it. Right now I would not feel comfortable with this going into the core without some more thoughts by others.

@Globulopolis
Copy link

Good idea! But getSibbling() not is too much. I think the class must work with the tree like with DOM tree and must have a "helper" methods to get next/prev/siblings/e.g.

* @return array The children
* @since 3.4
*/
public function &getChildren($recursive = false);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the & here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because you get an array and changes to that array should reflect back to the original node. I guess. I copied this from JCategoriesNode and I swear, it did make sense when I wrote that...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just reviewed this again and yes, this was simply to have the array by reference, so that we could remove children, etc.

@HermanPeeren
Copy link
Contributor

In general I like the idea to define a treenode interface, because we can then vary the tree-implementations (some implementations are more optimal for reading, others for writing).

BUT: wouldn't JTreeNodeInterface be more accurate?
What if you want to implement a more general graph, for instance a DAG, which is like a tree but can have multiple parents? How would you then call the interface of those nodes?

@Hackwar
Copy link
Member Author

Hackwar commented Dec 3, 2014

Hi @HermanPeeren, that is why I'm bringing this up here. We earlier had a JTree and a JNode and deprecated those... I honestly don't know the best way to go here. If nobody objects, we can rename it like that. I guess the only reason for me to call it JNodeInterface was to not have an empty folder named tree which only contains a folder named node in it... 😉

@HermanPeeren
Copy link
Contributor

Mmm, if you put it in hierarchical folders, then it would be called JNodeTreeInterface as a node of a tree is a special case of a more general node. And the superfolder doesn't have to be empty, for could have a definition of JNodeInterface. You would then have a JNodeTreeInterface extends JNodeInterface. The tree would then implement the restiction of a maximum of 1 parent.

Although that looks attractive at first sight, it is an endles road, for you could insert several other folders in between, like e.g. JNodeDAG (Directed Acyclic Graph). Besides that, there are node types that won't fit into such hierarchical type relations.

BTW: in the HTML DOM we also use the word node for tree nodes. The difference I see with a node in Joomla is, that the DOM is a tree and the word node to describe a part of the DOM is per definition only a tree node.

A way out of this swamp would be to just look to what we need and leave the rest out: you only want to better define trees in Joomla (hurray!), so you could call your node a a JTreeNodeInterface. Just as you did with your JNodeInterface, no empty folders or whatever, but just JTreeNodeInterface instead of JNodeInterface, leaving room for other node-types. The subject that you define, a tree, is the foldername: /tree. In that folder you can have a JTreeNodeInterface in libraries/cms/tree/nodeinterface.php. When using namespaces you would have some Joomla\Tree namespace: a package in which interfaces and implementations of tree-like structures are defined. And as we have seen with the DOM, within the scope of tree-types it is perfectly OK to talk about nodes then.

@Hackwar
Copy link
Member Author

Hackwar commented Feb 1, 2015

I've updated the PR to pass in Travis and I also updated the docs a bit. I would be happy if I could get some more input. 😄

@photodude
Copy link
Contributor

👍 I like this concept

@roland-d roland-d closed this May 8, 2016
@roland-d
Copy link
Contributor

roland-d commented May 8, 2016

@Hackwar Thank you for bringing this up but there seems to be no further interest at this point. Therefore I am going to close this PR but it can be re-opened when the topic is going to be revisited.


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/5227.

@Hackwar Hackwar deleted the nodeinterface branch April 27, 2019 07:47
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

Successfully merging this pull request may close these issues.

None yet

7 participants