More consistency...

[ccazette]

Hi there,

Just would like to highlight a few easy changes to improve Jelly consistency :

• Some classes use methods when some others use properties for the same functionalities : Jelly_Field::$model does the same as Jelly_Meta::model()
• Behaviors should behave like fields : data passed by the constructor is protected and a underscore is prepended, when all public in fields. Also Jelly_Meta has a field() method to allow setting / getting just one field, it should have a behavior() method just the same. That would allow to check easily if a model has a given behavior or not.
• Writing behavior methods is a pain because of arguments order... If you were to pass an extra argument to a behavior function, it is in between the model and the event data, which makes optional arguments a pain to handle...

Ex :

public function model_call_test(model, $optional, $data = NULL)

Why not simply :

public function model_call_test($model, $data, $optional1 = NULL, $optional2 = NULL)... ??

Some protected data miss method accessers, which limits flexibility :

• Jelly_Event should have a public method to check if a given event is defined, that would run an array_key_exists in the protected property _events
• Jelly_Builder should have a method to check if a given field is already in the the select, where, join or others statements. This is really an issue when writing behavior, as I ran into the same issue than @ivank in Paranoid behavior and Jelly_builder #95. May I suggest a has() method in the builder :

:

public function has($property, $field)
{
// Run some formatting on $field such as if field is an instance of Jelly_Field take the column name, if it is a text we should separe model and field name, it no model default to the builder model etc etc
switch($property)
{
case 'select' :
// check $this->_select
case 'where':
// check $this->_where
etc etc
}
}

Another (dirtier) option would be to make a __get() method to access these protected data

[ivank]

I second all those points - this all seems easy to implement and will cover a lot of the pain in writing behaviors.
However I still think behaviors should be able to save state per model instance. But I'm not sure how we can implement that (the current arrangement is very performant as there's only one behavior per class). Maybe allow some sort of array datastore for the model specific to behaviors

[ivank]

This can probably be the time to add a new "2.0" branch or something for breaking functionality

[ccazette]

Just want to add another one this :

• All Jelly_Fields should use parent functions. To take an example, the last line of Jelly_Field_Timestamp::save() should return parent::save($model, $value, $loaded); rather than return $value. Same goes for set() and get() method. This doesn't change anything as by default the field class returns the value unchanged but becomes a problem when you want to extend Jelly_Field behavior : your changes don't apply to fields not calling parent.