Thinking about the model
Due to a cold I had some time to think in the last few days (besides drinking tea, sleeping a lot, and watching videos). One thing I thought about was the Model class and how it could be improved.
If you look carefully at the API of the Model class, you will see that the current implementation assumes quite a bit about your models which doesn’t have to be true. So it assumes that you want to be able to find, update, save, and delete records through the model. Even though that’s often the case, it’s not always true. For example, you may want to have a read-only model. It’s something you cannot implement elegantly with CakePHP, as your model will always inherit methods like deleteAll(), save(), etc., and so those methods are part of the API of your model…
There are at least two possible ways to improve this.
The first approach is to make all such methods in Model protected. That means you could use those methods only from within your models. If you want to provide a delete-method, you would have to explicitly define such a method in your model and to delegate to the appropriate method from Model. The disadvantage of this approach is that it probably breaks a lot of code…
The second idea is to split the aforementioned functionality of the model into different behaviors. When writing your own model you can then choose which behaviors your model should have. For example, your model would use a “deletable” behavior if it should allow to delete records.
I am sure there are better ways than those I described here. They are simply thought as an inspiration to think about improving the model and to make it less fat ;-)
IMHO I prefer to use the second approach .. although I still didn’t know how to realize this .. but it is good thought:)
Wouldnt a simple behavior fix this ?
class SecurityBehavior extends ModelBehavior {
function settings(AppModel $model, $settings = array()) {
$this->settings[$model->alias] = am(array(’allowDelete’ => true, ‘allowSave’ => true), $settings);
}
function beforeDelete(AppModel $model) {
return $this->settings[$model->alias]['allowDelete'];
}
function beforeSave(AppModel $model) {
return $this->settings[$model->alias]['allowSave'];
}
?>
Above is extreme pseudo code, not syntax checked :)
I would definitely prefer the second idea (using behaviors). You could possibly flip the coin to get read-only Models or WORM Models and similar special cases through behaviors.
When I need a read-only model I currently let the beforeSave() and beforeDelete() return false. As I understand it, this functionality can be put into a behavior.
(haven’t started looking into behaviors much yet so if my assumptions are off then I’m sorry)
Wouldnt it work just to set up a empty (PHP5) protected method beforeDelete() in the class to avoid the execution of default code?
I’m afraid your approach here is wrong in several ways. First, the use case you’re describing (a read-only model) is very much in the minority. For us to make that the default case would be silly.
Also, implementing a “read-only” is laughably simple, since you only need to override a few methods and make them return false. This is yet another reason why making those methods protected is a silly idea.
Your idea that models should not be “fat” is also incorrect. The purpose of the model is to remove as much business logic as possible from the controllers, such that it is reusable. Hence “fat” models and “skinny” controllers.
The only case where this seems sensible (vis-a-vis your ticket) is making query() and execute() protected, as this would force people to put all their SQL in the model.
@all: Thanks for your comments!
@Martin: Yes, you could “solve” it with a behavior, see Christian’s comment.
@nate: Sure, I can override some methods and make them return false. That’s no problem. But it is a workaround, and the methods are still there. If I have a “read-only” model then I don’t want that my model has public methods like deleteAll() etc. But maybe that’s only me who thinks that’s not very elegant…
I fully agree with you on the principle of “fat” models and “skinny” controllers. With “to make it [the model] less fat” I meant the public interface of the Model class from the core.
Anyway, I hope you will rethink the Model a bit for the PHP5-based Cake2 ;-)
Couple things i would’ve foreseen in cake2
- Clean up all those public methods, most of them should be protected, Clean API is really needed break BC API if that matters.
- make Model::$data protected, a pair of getter/setter magic method for the fix.
- Return object instance, (i actually implemented this as a behavior)
Let me make sure I correctly understand what you’re suggesting. So, for a *normal* application (i.e. the default, conventional data-driven web app, the type of application for which Cake was designed in the first place), I’m usually not going to want read-only models. So for those cases, I really *would* want methods like save/saveAll, delete/deleteAll available to my controllers. In which case I’d have to override them and make them public for every single model that I wanted to not be read-only.
“Anyway, I hope you will rethink the Model a bit for the PHP5-based Cake2 ;-)”
I would be more than happy to, if you can explain how what you’re proposing reflects the philosophy of CakePHP. Because as I understand it, making save/delete protected would be an *edge case*, not something to implement as the default. And the idea that overriding a method is somehow ugly as you suggest, is patently ridiculous.
I agree with nate : such a behavior should be implemented by overriding the methods, either to trigger an error or to make them return false (depending on how bad you think it is to try to write to a read-only model). That’s not a workaround, that’s exploiting object-orientation.
That’s what is done in many classes of the standard Java API. For example, the Collection.remove() method is optional and may throw UnsupportedOperationException.
I’m not saying Cake should go that way because Java did, but it’s a common and lightweight practice.
FWIW - CakePHP is a general purpose model which allows general use cases to be accomplishes faster than not so often needed uses.
Besides as Nate writes - a read-only model is very simple to implement, if you feel that most of your models are going to be read-only write the code for it in app model…
I’ve seen silimar stuff in the Hibernate framework, but I would leave it as is. It would make things harder to understand for beginners.
I also think it is not very suitable for this framework.
@all: Thanks for your comments!
@speedmax: Yes, returning object instances would be cool to have! And I agree with you that the API should be cleaned up, and that many methods should be protected.
@nate: Yes, “writable” models are the default use case. But I think in most applications you also have “read-only” models, e.g. a menu.
Making methods like “deleteAll” protected is one possible solution. It is also possible to solve it with an inheritance structure where Model extends some generic model class which doesn’t provide methods to change anything. But the implementation is not that important at this point, it is more important to think about the model on a conceptual level. If you look from this point of view at the current implementation, you will see that it allows only a subset of the possible models. That means you have to override methods if you want to implement a model which is not in this subset, i.e. you try to solve a conceptual “problem” at the code level.
I hope it is now more clear what I mean ;-)
@Xr: Well, the OO way would be to have no such method if the method is not needed ;-) That’s of course the theory, in practice this is not always possible and you have to do what you described. If you have to override one method, that’s ok, but if there are x methods you have to override, then it smells…
@Tarique: For me models represent the objects from my problem domain. If something is read-only on the conceptual level, then there shouldn’t be methods for saving/updating/deleting in my implementation/API.
@Tim: This is just something I think *could* be improved in some future version, it doesn’t necessarily mean it *should* be done in that way.
I think it helps to question the existing solution from time to time, as that’s the only way to improve it ;-)
While we’re on changing models..
If I were writing cake .. I would have the models follow the singleton patterm. The way data is retrieved in cake there is no need for more than one instance of a model. I believe this is psuedo implemented with the ClassRegistry class.
@rtconner: Singleton is evil ;-) And if you look at what a model represents, a single data record (at least in theory), then you will see that it isn’t a singleton.
[...] Thinking about the model also forced me to think about the way I use and write models. This changed quite a bit over time. [...]
@cakebaker - i implemented ObjectizeBehavior specifically for that. I started working as a wacky experiment to create a data access api since object instance allows better encapsulation and access control.
My goal is to pass this object straight to Views (in my case templates)
the goal is so following is possible in my templates
page is a object instance returned by Page model,
{{ page.title | capitalize }}
or even
{{ page.get_related_pages.first.title }}
where get_related_pages is a method in Page model return list of page objects, print out first item’s title. (yes that was django syntax in php)
there is my very rough post about the details ObjectizeBehavior, sorry for poor writing but interesting stuff
http://idealian.net/?p=3
gwoo has a simpler approach
http://bin.cakephp.org/saved/1237
@speedmax: Thanks for the links, they look interesting. I think the challenge with passing models to the views is to not “abuse” them. Should you do something like get_related_pages() in the view or in the controller?
oh guys. I think you should think about this question much more earlier. I strongly recommend you to check out this espisode about how Rails expert suggests to do:
http://railscasts.com/episodes/4
@RainChen: Thanks for the link!
@cakebaker - What i was doing was a experiment after all. Its a very blur line in MVC architecture, for instance, Model represents data along with its data structure and business logic.
sorry I didn’t clarify it, It is not passing raw Model instance back, instead its passing a instance of custom data container “DataObject” class, which happens to have hidden reference to respective Model..
for the “abuse” issue.
You have to explicitly define and allows which Model methods is a callable in the object instance(DataObject Class) http://bin.cakephp.org/saved/25937 scroll down to see the implementation of DataObject class,
Its very similar to attr_reader in ruby, unless you have define them, methods are all private, (make it pretty hard to abuse, since abuse type coder are lazy)
for instance, Person::is_healthy() method hide underlying detail of person’s health record, but perform logic according to his (medical record, current health status) to determine and return a boolean. its a prefect example of leaving these logic inside model, (can be calculated inside controller, what happen if there are many of those, you see? i am trying to do something different)
Then the person object is very useful in view.
{% if not person.is_healthy %}
dude you have a problem
{% endif %}
PS: the syntax is H2O template engine i authored. a whole shebang of django port but compiles down to native php, very fast. if you interested. svn.idealian.net/h2o/branch/0.2
@speedmax: Ah, I overlooked that your solution uses DataObjects :| But it makes sense to use such a special data container object instead of the model.
[...] while ago I wrote an article Thinking about the model in which I shared some ideas about how CakePHP’s Model class could be improved. In this [...]