creocoder / yii2-nested-sets

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

Convert NestedSetsQueryBehavior to a trait #67

Open mikehaertl opened 9 years ago

mikehaertl commented 9 years ago

As traits are more efficient and you don't attach to any events in the NesteSetsQueryBehavior I think it could also be implemented as a trait.

creocoder commented 9 years ago

I already implemented is as trait as experiment. It works fine. I think its good idea to wait till 2.1 version of framework when behaviors will be replaced as traits.

mikehaertl commented 9 years ago

I think its good idea to wait till 2.1 version of framework when behaviors will be replaced as traits.

OT: Oh, didn't know that. Do you have some pointers where this is discussed, e.g. how will they implement attaching to events from multiple attached traits?

creocoder commented 9 years ago

@mikehaertl Unfortunately the discussion was in non relevant topics. But i think its good idea to create clean issue for such discussion inside Yii 2 repo.

mikehaertl commented 9 years ago

But back on topic: I'm not sure, if you really should wait for Yii 2.1. The query behavior could be converted right now, so why not already do it? Traits are more efficient, and in a typical app there will be way more queries than updates (where you need the nested set behavior). So why not improve that a little right now? And because your latest Version is 0.9.0 this minor BC change would be acceptable IMO.

Up to you of course.

teghnet commented 9 years ago

+1 to releasing the NestedSetTrait