creocoder / yii2-nested-sets

The nested sets behavior for the Yii framework.
Other
446 stars 129 forks source link

Let methods optionally accept a node id instead of a record #66

Open mikehaertl opened 9 years ago

mikehaertl commented 9 years ago

For more convenience it would be nice, if the tree related methods would also accept an id (or a Pk value to me more precise) instead of a loaded record. The behavior would then automatically load that record.

creocoder commented 9 years ago

Not sure about that. Can you show example when it may be interesting?

mikehaertl commented 9 years ago

There are many scenarios. It's really just for convenience: If I want to move a node somewhere else, why should I need to load the target node, too? It should be enough to call $node->appendTo($id).

In fact, even a static interface would be interesting, something like MyModel::appendNodeTo($id, $targetId); where both $id and $targetId could be anything, object or id. The instance methods could then utilize those static methos.

I found this way of implementing operations very convenient in my projects. It can even help to reduce the number of DB queries sometimes (you may not really need the full node instance for each operation - sometimes the id may be enough).

creocoder commented 9 years ago

If I want to move a node somewhere else, why should I need to load the target node, too? It should be enough to call $node->appendTo($id).

you may not really need the full node instance for each operation - sometimes the id may be enough

You always need full node instance. So even if you'll provide only id it will fetch node to get necessary data for operation. So since you can't reduce DB queries it always interesting to have target node instance in upper level for some additional checks. Long time ago (NS for Yii 1) i though about such methods, but i did not find any real world cases when you not interested in target node object in upper level. I'm not say there is no such examples. But even if so we need set of separate methods like appendToByPk, prependToByPk, etc...

mikehaertl commented 9 years ago

Ok, I see you point. This is really only for a bit more convenience. No problem if it's not implemented.