jonathangeiger / kohana-jelly

See the link below for the most up-to-date code
https://github.com/creatoro/jelly
MIT License
146 stars 34 forks source link

Quick thoughts on Jelly 1.0 #133

Closed ccazette closed 14 years ago

ccazette commented 14 years ago

I've tried Jelly 1.0 and think it is near to be stable really ! Awesome work, this release brings a lot of enhancements to a already great library... A few remarks, in disorder :

Thanks again for your work !

ccazette commented 14 years ago

Oh, and I am also wondering why you've re-implemented where ID IN (subquery) instead of JOINS for ManyToMany, while you had implemented JOINS on Jelly 0.x. JOINS are slightly faster and allow to get additional columns for the "through" table.

banks commented 14 years ago

I won't comment on the first too as I've not looked closely enough at that code to comment usefully now.

On the third point.

but the goal seems to be about the same to me

There are very clear differences. I've not look that hard at the implementation but in conceptual terms the model builder and a behaviour are comepltely different things.

Model specific builder extensions are for encapsulating listing logic; they are for enhancing the query builder with domain-specific methods for querying your applications model graph. They allow you to manipulate (or generate) lists of models in an encapsulated, flexible and natural way.

Behaviors allow you to add functionality to model instances (as well as model builders) in such a way that multiple inheritance is achievable. For instance one model may have MPTT ordering AND be version controlled simply by declaring that it displays both behaviors. Another model may only support MPTT without convoluted inheritance trees or code duplication.

The fact that a behavior can add enhancements to the model builder may blur the lines slightly but this is simple so that if you declare that your model behaves in a certain way, that can imply certain functionality both for instances of that model and lists of that model. For example, I may write several different models that all share some functionality such as maintaining a published/unpublished status. Rather than duplicate all the fields/code for handling this status, I may just create a behaviour call Jelly_Behavior_Publishable which adds functions to the instance like is_published() or publish_now() but could also add functions to the builder like published(TRUE) so I can do things like Jelly::select('post')->published(TRUE) and not have to duplicate the publishing related functions in multiple models.

Hope that clears that up a bit.

ccazette commented 14 years ago

Hi Banks, thank you very much for your thorough explanation. That clears things up for me ! Thank you for your time I hope it'll be helpful for others.

The other points, more technical, remain valid though...

jonathangeiger commented 14 years ago

Oh, and I am also wondering why you've re-implemented where ID IN (subquery) instead of JOINS for ManyToMany, while you had implemented JOINS on Jelly 0.x. JOINS are slightly faster and allow to get additional columns for the "through" table.

Basically, because it allows for:

$post->get('tags')->delete();

This isn't possible with a JOIN, but with a sub-query, it is. I took a look at that thread and the performance gains didn't really seem that significant. I'd rather have the API be consistent.

ccazette commented 14 years ago

Humm... That is pretty annoying as many "through" tables have additional columns, such as a date or any other information on the relationship itself. JOINS allowed to get them from the same query, which was the most important point for ManyToMany to use them. Guess I'll have to extend the builder. Right, so only my first 2 remarks on Fields_Enum and Field File remain valid so far...

jonathangeiger commented 14 years ago

Enums should accept associative arrays. Partially closed by 62b50431ae1cd4da49ae30dfd37fb41ae92e53e2