CakePHP Programming

Best practice for dealing with additional data in beforeSave()

On somebody asked how to securely set a value inside a form. Besides the obvious fact that you don’t do that if the value is not needed for some purpose in the view, one of the answers given to that question contained so much not so good code, that it inspired me to give a detailed answer as well and to write this article about it. Here is the code in question:

Short summary of what is wrong here:

  • $this->alias is not used
  • The method signature is wrong and will cause a php 5.4+ strict warning
  • Tight coupling with the Router class is introduced
  • The direct assignment of an integer value here is not good
  • The if check is not using strict === comparison

This code is introducing tight coupling with a static call and the model should not need to be aware of the Router. Also you’re missing the $options argument in the Model::beforeSave() which causes strict errors on php 5.4+ and up to date CakePHP. See this pull request for reference.

Further this couples the model code with a specific controller action and don’t even check for the controller as well. If the parameters are needed they should be passed to the model instead of accessed through a static call of the Router. In fact you can pass additional options through the second argument of the Model::save() method. The documentation doesn’t explain this very well, but if you look at the code you can pass any additional options besides the validate and callbacks options. These options are passed to the Model::beforeSave() method, which is in fact listening to the Model.beforeSave event. So in fact you could do this:

The best way to do it would be in the model because this will make sure the default group id gets always set, no matter from where the save is called and you can’t forget to pass the group id:

An alternative way is to  simply add the data in the controller method to $this->request->data but the model is the far better place because you can test the whole thing, use it in a shell and have everything in one place and the right place because all of this belongs into the model layer.