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

[8.x] Added class and method to create cross joined sequences for factories #40542

Merged
merged 4 commits into from
Jan 27, 2022

Conversation

thomasowow
Copy link
Contributor

@thomasowow thomasowow commented Jan 21, 2022

Adds a new CrossJoinSequence class that helps cross joining multiple sequences.

Usage:

User::factory(4)
    ->crossJoinSequence(
        [['first_name' => 'Thomas'], ['first_name' => 'Agent']],
        [['last_name' => 'Anderson'], ['last_name' => 'Smith']],
    )
    ->create();

// Results in

[
    ['first_name' => 'Thomas', 'last_name' => 'Anderson'],
    ['first_name' => 'Thomas', 'last_name' => 'Smith'],
    ['first_name' => 'Agent', 'last_name' => 'Anderson'],
    ['first_name' => 'Agent', 'last_name' => 'Smith'],
]

Current way of achieving this:

->sequence([
    ['first_name' => 'Thomas'],
    ['first_name' => 'Thomas'],
    ['first_name' => 'Agent'],
    ['first_name' => 'Agent']
])
->sequence([
    ['last_name' => 'Anderson'],
    ['last_name' => 'Smith'],
    ['last_name' => 'Anderson'],
    ['last_name' => 'Smith'],
])

More complex example:

new CrossJoinSequence(
    [['a' => 1]],
    [['b' => 1], ['b' => 2]],
    [['c' => 1], ['c' => 2], ['c' => 3]],
);

[
    ['a' => 1, 'b' => 1, 'c' => 1],
    ['a' => 1, 'b' => 1, 'c' => 2],
    ['a' => 1, 'b' => 1, 'c' => 3],
    ['a' => 1, 'b' => 2, 'c' => 1],
    ['a' => 1, 'b' => 2, 'c' => 2],
    ['a' => 1, 'b' => 2, 'c' => 3],
];

@taylorotwell
Copy link
Member

Should we name it CrossJoinSequence since it has a clear correlation with Arr::crossJoin? Or does that not make sense?

@Jubeki
Copy link
Contributor

Jubeki commented Jan 21, 2022

I like the idea. And I think both names MatrixSequence and CossJoinSequence are fine.

What is the expected behaviour of:

new MatrixSequence(
    [['a' => 1]],
    [['b' => 1], ['b' => 2]],
    [['c' => 1], ['a' => 2], ['c' => 3]], // Here is the second element changed.
);

// Later has a higher priority
[
    ['a' => 1, 'b' => 1, 'c' => 1],
    ['a' => 2, 'b' => 1],
    ['a' => 1, 'b' => 1, 'c' => 3],
    ['a' => 1, 'b' => 2, 'c' => 1],
    ['a' => 2, 'b' => 2],
    ['a' => 1, 'b' => 2, 'c' => 3],
];

// Earlier has a higher priority
[
    ['a' => 1, 'b' => 1, 'c' => 1],
    ['a' => 1, 'b' => 1],
    ['a' => 1, 'b' => 1, 'c' => 3],
    ['a' => 1, 'b' => 2, 'c' => 1],
    ['a' => 1, 'b' => 2],
    ['a' => 1, 'b' => 2, 'c' => 3],
];

What do you think about this alternative?

new MatrixSequence([
    'a' => [1],
    'b' => [1, 2],
    'c' => [1, 2, 3],
]);

// Results in
[
    ['a' => 1, 'b' => 1, 'c' => 1],
    ['a' => 1, 'b' => 1, 'c' => 2],
    ['a' => 1, 'b' => 1, 'c' => 3],
    ['a' => 1, 'b' => 2, 'c' => 1],
    ['a' => 1, 'b' => 2, 'c' => 2],
    ['a' => 1, 'b' => 2, 'c' => 3],
];

@taylorotwell taylorotwell marked this pull request as draft January 24, 2022 17:27
@taylorotwell
Copy link
Member

Marking as draft pending response to feedback.

@thomasowow
Copy link
Contributor Author

Should we name it CrossJoinSequence since it has a clear correlation with Arr::crossJoin? Or does that not make sense?

CrossJoinSequence was a second choice 👍. However now my Matrix example with Thomas Anderson and Agent Smith seems so unrelated 😿.

new MatrixSequence([
    'a' => [1],
    'b' => [1, 2],
    'c' => [1, 2, 3],
]);

This indeed feels like a nice alternative. It prevents human error and is much easier to read/understand. However for unknown reasons and possibly lack of knowledge about defining a matrix I don't feel that convinced about its signature. Have you seen this somewhere before?

I would at least rename the current class to CrossJoinSequence. Should I also add ->crossJoinSequence as helper method?

@thomasowow thomasowow changed the title [8.x] Added class to create matrix sequences for factories [8.x] Added class and method to create cross joined sequences for factories Jan 27, 2022
@thomasowow thomasowow marked this pull request as ready for review January 27, 2022 17:35
@Jubeki
Copy link
Contributor

Jubeki commented Jan 27, 2022

Sorry @thomasowow I forgot to answer.

I made my example like multiple foreach loops:

foreach ([1] as $a) {
 
    foreach ([1, 2] as $b) {

        foreach ([1, 2, 3] as $c) {

            Model::create([
                'a' => $a,
                'b' => $b,
                'c' => $c,
            ]);

        }

    }

}

@taylorotwell taylorotwell merged commit 22e32b9 into laravel:8.x Jan 27, 2022
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.

3 participants