Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support mixed target data in data_get #4061

Merged
merged 1 commit into from May 30, 2014

Conversation

JosephSilber
Copy link
Member

This will allow data_get to go many levels deep into an object/array, supporting objects/arrays interchangeably all the way down the tree:

$object = new stdClass;
$object->framework = ['name' => 'Laravel'];
$framework = data_get($object, 'framework.name');

@anlutro
Copy link
Contributor

anlutro commented Apr 7, 2014

Heya, I've written a function like this myself and have a couple of suggestions.

First of all, an object that implements ArrayAccess will return false for is_array and true for is_object, but that doesn't necessarily mean that $object->{$segment} will be set. If the object implements ArrayAccess you should do an additional check

if ($object instanceof ArrayAccess && $object->offsetExists($segment))
    return $object->offsetGet($segment);

I also think that Eloquent collection should receive special treatment - its array keys does not correspond to the models' keys, so I suggest we call find($segment) as opposed to offsetGet.

If this is merged then the form builder should definitely be changed to use this instead of array_get/object_get.

@GrahamCampbell
Copy link
Member

Also, this will need to go to the master branch, not 4.1.

@JosephSilber
Copy link
Member Author

@GrahamCampbell Why? Do you think this breaks BC? Passing a nested array or a nested object still works as expected.

@anlutro Sounds good, but that will make this function a bit unwieldy (how I wish we could do this in a proper class and just defer to it. Kinda like the way the string helpers are done). If @taylorotwell thinks we should still do it, I'll gladly add it (though I'm not sure about the model keys. Don't you think that's taking it a bit too far?).

@anlutro
Copy link
Contributor

anlutro commented Apr 8, 2014

I don't, primarily because I see the use of a function like this in model form binding, where proper handling of Eloquent relationships has been requested for a long time.

taylorotwell added a commit that referenced this pull request May 30, 2014
Support mixed target data in data_get
@taylorotwell taylorotwell merged commit 463670c into laravel:4.1 May 30, 2014
@JosephSilber
Copy link
Member Author

@taylorotwell what do you think about adding functionality for ArrayAccess and Collection@find?

@JosephSilber JosephSilber deleted the data-get-mixed branch June 15, 2014 03:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants