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.5] Add buildToDictionary Method #21505

Merged
merged 3 commits into from Oct 3, 2017

Conversation

Projects
None yet
2 participants
@ConnorVG
Contributor

ConnorVG commented Oct 2, 2017

I've came across the need to build a dictionary a few times and each time it's something ugly or overly busy (doing too much for what it is). I found the perfect use case, HasOneOrMany's buildDictionary method.

This extracts a bit of logic from the mapToGroups method; no extra code is added, it's just extracted.

See this Twitter chain for a bit of context: https://twitter.com/ConnorVG/status/913062913837060098

Please note, I haven't added tests for mapToDictionary as-of yet (though they're currently covered by mapToGroups, it will still need its own tests obviously).

Connor S. Parks added some commits Sep 27, 2017

@taylorotwell

This comment has been minimized.

Show comment
Hide comment
@taylorotwell

taylorotwell Oct 2, 2017

Member

Can you provide an example or description of what this does exactly?

Member

taylorotwell commented Oct 2, 2017

Can you provide an example or description of what this does exactly?

@taylorotwell

This comment has been minimized.

Show comment
Hide comment
@taylorotwell

taylorotwell Oct 2, 2017

Member

For example, can we add tests for mapToDictionary?

Member

taylorotwell commented Oct 2, 2017

For example, can we add tests for mapToDictionary?

@ConnorVG

This comment has been minimized.

Show comment
Hide comment
@ConnorVG

ConnorVG Oct 2, 2017

Contributor

Will do, need to take care of some work then I'll jump on it quickly.

Contributor

ConnorVG commented Oct 2, 2017

Will do, need to take care of some work then I'll jump on it quickly.

Connor S. Parks Connor S. Parks
@ConnorVG

This comment has been minimized.

Show comment
Hide comment
@ConnorVG

ConnorVG Oct 2, 2017

Contributor

Quickly added some tests based on the mapToGroups tests.

The best example I can think of, in terms of its functionality, is the dictionary mapping in HasOneOrMany:

https://github.com/ConnorVG/framework/blob/7d8d81693aaa18d07e3fe0cd6e380a7518361406/src/Illuminate/Database/Eloquent/Relations/HasOneOrMany.php#L163-L170

vs

https://github.com/laravel/framework/blob/5.5/src/Illuminate/Database/Eloquent/Relations/HasOneOrMany.php#L163-L176

I've used it locally for something similar, matching a bunch of Api results to a bunch of models based on a specific key (creator_id and proposal_id, in my case). This is done simply like this:
image

Contributor

ConnorVG commented Oct 2, 2017

Quickly added some tests based on the mapToGroups tests.

The best example I can think of, in terms of its functionality, is the dictionary mapping in HasOneOrMany:

https://github.com/ConnorVG/framework/blob/7d8d81693aaa18d07e3fe0cd6e380a7518361406/src/Illuminate/Database/Eloquent/Relations/HasOneOrMany.php#L163-L170

vs

https://github.com/laravel/framework/blob/5.5/src/Illuminate/Database/Eloquent/Relations/HasOneOrMany.php#L163-L176

I've used it locally for something similar, matching a bunch of Api results to a bunch of models based on a specific key (creator_id and proposal_id, in my case). This is done simply like this:
image

@taylorotwell

This comment has been minimized.

Show comment
Hide comment
@taylorotwell

taylorotwell Oct 3, 2017

Member

How is this different than mapWithKeys()?

Member

taylorotwell commented Oct 3, 2017

How is this different than mapWithKeys()?

@ConnorVG

This comment has been minimized.

Show comment
Hide comment
@ConnorVG

ConnorVG Oct 3, 2017

Contributor

The main differences are the same as the main differences between mapWithKeys and mapToGroups:

  1. It doesn't mutate based on the k/v, just the value;
  2. The callback returns the respective k/v pair to be set in the results, it doesn't return an array of k/v pairs.
Contributor

ConnorVG commented Oct 3, 2017

The main differences are the same as the main differences between mapWithKeys and mapToGroups:

  1. It doesn't mutate based on the k/v, just the value;
  2. The callback returns the respective k/v pair to be set in the results, it doesn't return an array of k/v pairs.

@taylorotwell taylorotwell merged commit 7d8d816 into laravel:5.5 Oct 3, 2017

2 checks passed

continuous-integration/styleci/pr The StyleCI analysis has passed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@ConnorVG ConnorVG deleted the ConnorVG:5.5 branch Oct 3, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment