Change what type of types can be assigned #12
Conversation
Allows an object, string or an array to be assigned to the storage array. refs: slimphp#7
| if ( | ||
| is_array($message) === true | ||
| || is_object($message) === true | ||
| ) { |
There was a problem hiding this comment.
I honestly would get rid of the checks and just have it append to the array without the (string)
//ping @silentworks @akrabat
There was a problem hiding this comment.
@geggleto Sorry, I was meant to have added a comment about this with the PR.
This was a discussion point more than anything as my thoughts behind this was that pushing an array or object onto the slimFlash stack with the given key would result in the following:
array(
'slimFlash' => array(
'key' => array(
0 => array(
'data' => array(
// ...
)
)
)
)
)This could become annoying trying to access the data stack.
Am I over thinking the problem or is it just creating an issue/hindrance?
There was a problem hiding this comment.
@NigelGreenway yeah I think I would leave that down to the developer to decide. I now understand why we were only storing string as the session store might not know how to deal with objects, but I also think if a developer decides to throw an object into the session, they should know what their store can handle or deal with how it should handle what they throw at it.
My expectation was to see this:
$this->storage[$this->storageKey][$key][] = (string)$message;become:
$this->storage[$this->storageKey][$key][] = $message;Here I would be making no assumption as to what the developer want to store, $message could be a string, object or array if they wanted. We might however need to note this in the README.
There was a problem hiding this comment.
@silentworks, @geggleto; I will change that now.
Makes more sense leaving it down to the developer.
It was suggested by @silentworks that the only change to the code base should be that `(string)` type casting should be removed and allow the developer to handle how its handled.
|
I have update the code base. @silentworks, would you have a look over the readme with my addition as I am not sure how to word it, unless what I have put is enough? |
|
Looks fine to me. |
…ages Change what type of types can be assigned
|
Thanks for this @NigelGreenway |
|
np @silentworks :) |
|
@silentworks, when will this be published to Packagist? The latest date for slim/flash on Packagist is 2015-08-16 22:49 UTC. |
|
+1 for tagging a release! |
|
I will poke some pepple for a release. |
|
Version 0.2.0 released to Packagist. Sorry - didn't realise that this one wasn't in 0.1.0 |
Allows an object, string or an array to be assigned to
the storage array.
refs: #7