Allow Closure generators to access generated attributes #311
Conversation
This allows generators of the 'Closure' type to access attributes immediately after they are generated via the object that is passed in as the first paramater.
| @@ -502,7 +498,7 @@ public function isPendingOrSaved($object) | |||
| public function deleteSaved() | |||
| { | |||
| $exceptions = array(); | |||
| foreach ($this->saved as $object) { | |||
| foreach (array_reverse($this->saved) as $object) { | |||
There was a problem hiding this comment.
Would it be more efficient for us to just add them to the starts of the saved array in the first place rather than reversing the entire array at this point?
Also, this is a change in behaviour, so should really be going into 2.2, rather than 2.1.
There was a problem hiding this comment.
This doesn't actually work in the way you expect it to. I'm pretty sure the order here is random since we're storing the objects with their hashes as keys, so php will iterate over them alphabetically by hash (so the order will appear random).
There was a problem hiding this comment.
|
One failing test here: That confirms what I said inline. This will have to go into 2.2 because of the change in behaviour. |
|
@pmccarren Can you send only your "Set the att value on the model once generated." change to the "experimental" (3.0) branch please. |
|
Sure, absolutely. |
|
Great. :) |
|
Good to go! :) |
|
@GrahamCampbell One possible way to remove objects in reverse order is once we create an object, we can append its objectHash (what spl_object_hash generates) to a reference array, and then when needed, simply iterate over said array in reverse order. The one check you'd have to have is to ensure that the object still exists before you attempt to delete it. Thoughts? I'm happy to prototype this too |
|
Yeh, would be nice, but would lead to poor performance of the isSaved function. I assume you mean something like this under the hood: $saved = array(
0 => array('our hash', object),
1 => array('our hash', object),
2 => array('our hash', object),
); |
|
Hmmm. What if we were to simply have 2 completely separate arrays. That could be the answer! |
$saved = array(
'hash a' => object3,
'hash b' => object1,
'hash c' => object2,
);
$map = array(
0 => 'hash b',
1 => 'hash c',
2 => 'hash a',
); |
|
I'm going to get something to eat. I'll be back in 30 mins or so, and I'll take a look at this. Feel free to tackle this in a pull to 2.2 @pmccarren. |
|
@GrahamCampbell yeah exactly! sorry I wasn't clear there. I believe two separate arrays is the key |
|
Prototyped it over here: #316 |
|
Hmmm. I did want it in 2.2, rather than 3.0, but leave the pull open anyway so we can discuss it. |
|
Would you like me to create a PR with this change into 2.2/master? |
|
No, don't bother. When I've got time, I'll review your change against 3.0. Thanks for taking the time to contribute - it's much appreciated. :) |
|
No problem! Glad to help out. using FactoryMuffin has made my testing experience so much better |
No description provided.