fago and I did a first small code review and we noted some points which should be improved:

* use 2 variables instead of $children and rename left/right to trueTree/falseTree
* prefix the name of the treenode class with your module, e.g. RulesOptimizationTreeNode
* this is not necessary:

$rules = $event->getIterator();
foreach ($rules as $rule) { ... }

just use foreach ($event) {} (getIterator() is a magic PHP method).

* arrayCopy(): you are cloning objects there, so rename it to arrayClone or deepClone; even better: copy the code to a magic __clone() method for ContainerPlugin as a patch against rules
* implement necessary methods for RulesPlugin: Rule class as example (e.g. integrityCheck() that works on children also)
* comment on the $data property of TreeNode, what is in there? condition?
* maybe TreeNode should extend CopntainerPlugin or ActionContrainerPlugin where $children is implemented already
* don't save the actions to $children, create a separate property for it to make it more clear
* $isLeaf property is not needed, as you can just implement a method isLeaf() where you check if trueTree and falseTree are set

Comments

Saubhagya’s picture

Saubhagya commented

Some improvements are to make code easy to understand, so I'll implement them, but

I didn't get this one:
> implement necessary methods for RulesPlugin: Rule class as example (e.g. integrityCheck() that works on children also)

Which l Rule class methods I need to implement in TreeNode and why? And if integrityCheck() fails at any TreeNode, should I halt the execution?

> maybe TreeNode should extend CopntainerPlugin or ActionContrainerPlugin where $children is implemented already

For this I need to override some methods, so I'll go through 2 variables strategy:)

> don't save the actions to $children, create a separate property for it to make it more clear

I'm taking an action array in TreeNode, which is empty for conditions and have actions at leaves.