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

[5.3] Ability to use keyBy on objects castable to string #16001

Merged
merged 1 commit into from
Oct 22, 2016

Conversation

andrewfenn
Copy link
Contributor

Ran into an issue specifically where I couldn't conveniently use keyBy on a collection with a Carbon object.

The proposed change allows one to use keyBy against any object that can be cast to a string without modifying the existing functionality.

To give an example, say we have the following code which is typical if you're using Carbon dates in laravel or lumen on eloquent models..

        \Carbon\Carbon::setToStringFormat('Y-m-d');

        $obj = new \stdClass;
        $obj->date = new \Carbon\Carbon('2017-09-01');
        $obj->name = 'hello';
        $test = new \Illuminate\Support\Collection([$obj, $obj]);

       // Illegal offset type Exception called
       dd($test->keyBy('date'));        

The expected output that this patch provides

object(Illuminate\Support\Collection)#134 (1) {
  ["items":protected]=>
  array(1) {
    ["2017-09-01"]=>
    object(stdClass)#117 (2) {
      ["date"]=>
      object(Carbon\Carbon)#130 (3) {
        ["date"]=>
        string(26) "2017-09-01 00:00:00.000000"
        ["timezone_type"]=>
        int(3)
        ["timezone"]=>
        string(3) "UTC"
      }
      ["name"]=>
      string(5) "hello"
    }
  }
}

Ran into an issue specifically where I couldn't conveniently use keyBy on a collection with a Carbon object.

The proposed change allows one to use keyBy on against any object that can be cast to a string without modifying the existing functionality.
@GrahamCampbell GrahamCampbell changed the title Ability to use keyBy on objects castable to string [5.3] Ability to use keyBy on objects castable to string Oct 19, 2016
@taylorotwell
Copy link
Member

Why even check if it's an object? Why not just cast to string every time?

@andrewfenn
Copy link
Contributor Author

At the time i was thinking you might want the keys to be other values such as int, casting to string would lose that? Whatever works really is fine by me.

@taylorotwell
Copy link
Member

Yeah I guess that is true.

@taylorotwell taylorotwell merged commit 482efe9 into laravel:5.3 Oct 22, 2016
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

2 participants