Model::save() now returns an array!
Posted on 3/11/07 by Felix Geisendörfer
Just got bitten by this one when updating to the latest version of CakePHP. If you use code like this in your app:
// Do stuff
}
Then you're in for a surprise. Because as of revision 5895 Model::save() now returns Model::data on success if its not empty. Now most of us do not use strict comparison for checking the return value of Model::save(), but I was stupid enough to do it as part of my new "fail hard fast" strategy : ). So suddenly I had stuff blowing up all over the place.
Oh well, thats the price you pay for being one of the cool kids and staying with SVN:HEAD. However, its definitely worth it because there are new goodies added to the core every day. And in fact, Model::save() returning an array is one of them. Because previously you had to make your own copy of Model::data if you needed it after a save() operation because Model::save() always clears Model::data for you.
So its all good, just be sure to check your app for strict Model::save return value comparisons to avoid surprises.
-- Felix Geisendörfer aka the_undefined
Did you like this blog post? If so, please consider subscribing to the Blog RSS feed.
You can skip to the end and add a comment.
Just one gotcha - I also thought this should return the new insert ID if a record was created - as part of the data array (i.e. as if you had just done a 'find()') but apparently not...
https://trac.cakephp.org/ticket/3481
Still, I'm extremely pleased we now get most of the model data back.
~GreyCells
Spout: Your if statement will work.
GreyCells: Thanks for pointing this out.
I'm still confused on cake's implementation of ActiveRecord. IMO, this hack is a step in the *wrong* direction. Why not persist the data in Model::data()? GreyCells ticket was answered that Model::id stores the PK on save... so the instance carries the reference to the record in the DB - seems to me that the most logical thing would be to carry the data of the saved record too (all the data, not just the modified columns).
An annoyance for me is within afterSave() when you need to check the existence of a column then fetch the field if it's not set.
if (!isset($this->data[$this->name]['some_column'])) {
$this->data[$this->name]['some_column'] = $this->field('some_column');
}
// Use $this->data[$this->name]['some_column']
Also, what about afterDelete() callback? I run into many situations where I need foreign key values of a deleted record (counterCache behavior, etc).
Felix Geisendorfer's Blog: Model::save() now returns an array!...
Felix Geisendorfer has a quick tip for CakePHPers out there today ......
[...] Felix Geisendorfer has a quick tip for CakePHPers out there today - an update to the framework that might cause a “gotcha” moment in your code: Just got bitten by this one when updating to the latest version of CakePHP. If you use code [checking to see if the return from a save() is true] in your app, you’re in for a surprise. Because as of revision 5895 Model::save() now returns Model::data on success if its not empty. [...]
@Brandon P: I'm still unsure as to why the latestInsertId does not get set in the returned array, it seems most illogical to me, but as a non-contributor (apart from tickets) I/we don't necessarily get to influence such decisions.
I don't know the historic reasons for clearing the Model instance's data array after a save - it may be to do with the use of a single instance for all database interaction (as in $this->ModelName->save($dataArray) rather than creating a new instance per record processed. In other words, by convention, we do $newDataArray = $this->ModelName->create() rather than $newModelInstance =& new ModelName().
~GreyCells
@Brandon P: Your wish may come true in v2.0 (from the trac roadmap):
"Change Models to return object instances instead of arrays"
:)
Folks: Truths is, CakePHP does not "really" use ActiveRecord. By that I mean it doesn't implement it in the same way lets say Ruby on Rails does. And in fact, this is something I'm happy about right now. I have experiences with apps using ~300 models (in PHP5) and I tell you - this is when things become somewhat sluggish. Now imagine iteration through an array of 1000 records when each of them is a record and holds associations to all models its associated with - CRAZY.
Now of course, there are ways to optimize things by simply making those returned models more light weight and mostly de-coupled from other models. But again, there are lots of advantages with the way CakePHP is doing things right now.
@Brandon: I send you an email, please get back to me.
@Felix: You're right, 'active record' implementation is very efficient (using a variation of the 'Flyweight' design pattern?). I'm hoping that version 2 will not stray too far from this e.g. keep anything that is common to all classes (such as associations) in a singleton and pass a reference to that when calling methods on the 'data' instance. A bit like 'super behaviours'.
~GreyCells
P.S. Any chance of a sneak preview of PostTask.com?
this is a hacking skill?
because it's not in the trunk of svn.
This post is too old. We do not allow comments here anymore in order to fight spam. If you have real feedback or questions for the post, please contact us.
I use if(Model->save()){} is it good? Or have to test with empty()?