-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
[Variation] Reusable Set Builders for generating variants + VariantGenerator spec coverage #2198
Conversation
Nice, let's wait for the build. :) I think it is not a problem if someone does not know what a Cartesian product is. With these changes, the design is better and generation easier to change, so 👍. |
@@ -0,0 +1,62 @@ | |||
<?php | |||
|
|||
namespace Sylius\Component\Variation\SetBuilder; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing license note.
44a8987
to
8d0c583
Compare
@stloyd Thanks, I've addressed your comments and have squashed the commits. |
|
||
if (1 === $countArrays) { | ||
return reset($setTuples); | ||
} elseif (0 === $countArrays) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be just if
as you call return
in previous one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
8d0c583
to
4fa52f6
Compare
@pjedrzejewski ping... |
[Variation] Reusable Set Builders for generating variants + VariantGenerator spec coverage
Thank you Adam! 👍 |
[Variation] Reusable Set Builders for generating variants + VariantGenerator spec coverage
Whilst working on writing specs for the
VariantGenerator
, I could see why there weren't any to begin with.Now the most complicated logic has been extracted out to a new class
CartesianSetBuilder
, which implementsSetBuilderInterface
, so this has the added benefit of allowing developers to define their own strategies for generating variants.Unfortunately, the disadvantage of this being that the set builder has to be explicitly passed to the constructor of
VariantGenerator
, and only math boffins would be aware of what a set builder and the Cartesian product are.