Skip to content

allows passing an enum to randomElement() or randomElements()#620

Merged
localheinz merged 15 commits intoFakerPHP:mainfrom
cosmastech:feature/allow-random-element-from-enum
May 14, 2023
Merged

allows passing an enum to randomElement() or randomElements()#620
localheinz merged 15 commits intoFakerPHP:mainfrom
cosmastech:feature/allow-random-element-from-enum

Conversation

@cosmastech
Copy link
Copy Markdown

@cosmastech cosmastech commented Mar 29, 2023 •
edited by localheinz
Loading

What is the reason for this PR?

faker()->randomElement(SomeEnum::cases())

is fine, but I think it would be even nicer to have

faker()->randomElement(SomeEnum::class);
  • A new feature

Author's checklist

Summary of changes

Review checklist

  • All checks have passed
  • Changes are approved by maintainer

@localheinz localheinz changed the title allows passing an enum to randomElement or randomElements allows passing an enum to randomElement() or randomElements() Apr 1, 2023
@localheinz localheinz changed the title allows passing an enum to randomElement() or randomElements() allows passing an enum to randomElement() or randomElements() Apr 1, 2023
@cosmastech
Copy link
Copy Markdown
Author

cosmastech commented Apr 6, 2023

@localheinz is there I need to do with the failing pipeline items? The enum_exists issue is already accounted for by checking if the function exists.

Didn't look into the \Traversable thing too closely.

@stale
Copy link
Copy Markdown

stale bot commented Apr 25, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 1 week if no further activity occurs. Thank you for your contributions.

localheinz
localheinz reviewed Apr 27, 2023
View reviewed changes
localheinz
localheinz reviewed Apr 27, 2023
View reviewed changes
localheinz
localheinz reviewed Apr 27, 2023
View reviewed changes
localheinz
localheinz reviewed Apr 27, 2023
View reviewed changes
@localheinz localheinz self-assigned this Apr 27, 2023
@localheinz localheinz added the enhancement New feature or request label Apr 27, 2023
localheinz
localheinz approved these changes Apr 27, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@localheinz localheinz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can simplify this a bit, but it looks good to me otherwise!

@cosmastech
Copy link
Copy Markdown
Author

cosmastech commented Apr 27, 2023

I think we can simplify this a bit, but it looks good to me otherwise!

Thanks @localheinz!

@localheinz localheinz force-pushed the feature/allow-random-element-from-enum branch from eea2bd5 to f58c881 Compare April 27, 2023 13:48
cosmastech
cosmastech commented Apr 27, 2023
View reviewed changes
src/Faker/Provider/Base.php Outdated
$arr = $array;

if (is_string($array) && function_exists('enum_exists') && enum_exists($array)) {
$traversables = $array::cases();
Copy link
Copy Markdown
Author

@cosmastech cosmastech Apr 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we change this to $arr = $traversables = $array::cases() the test passes, but I'm still unsure why it was failing in the first place

@localheinz localheinz mentioned this pull request Apr 27, 2023
Merged
5 tasks
@localheinz
Copy link
Copy Markdown
Member

localheinz commented Apr 27, 2023

@cosmastech

I find the code hard to understand in the first place and I am trying to clean it up in #639!

@localheinz localheinz force-pushed the feature/allow-random-element-from-enum branch from 2f21b41 to 3ac0b61 Compare May 1, 2023 10:36
@localheinz localheinz mentioned this pull request May 1, 2023
Merged
1 task
@localheinz localheinz assigned pimjansen and bram-pkg and unassigned localheinz May 1, 2023
cosmastech and others added 11 commits May 2, 2023 10:07
@cosmastech @localheinz
specify class-string
5246503
@cosmastech @localheinz
Update test/Faker/Provider/BaseTest.php
674e29c 
Co-authored-by: Andreas Möller <am@localheinz.com>
@cosmastech @localheinz
remove require_once
e0fcd03 
Co-authored-by: Andreas Möller <am@localheinz.com>
@cosmastech @localheinz
clean up tests
a61341d 
splits tests, splits phases of tests, adds more specific assertions

Co-authored-by: Andreas Möller <am@localheinz.com>
@cosmastech @localheinz
Apply suggestions from code review
b15f47c 
Co-authored-by: Andreas Möller <am@localheinz.com>
@localheinz
Fix: Avoid unnecessary import
1116450
@localheinz
Fix: Remove unnecessary reference to root namespace
6730fe4
@localheinz
Fix: Exclude BackedEnum
21544ff
@localheinz
Fix: DocBlock
9d9e917
@localheinz
Fix: Run 'make baseline'
6b378a9
@localheinz
Fix: Run 'make baseline'
021e8d3
@localheinz localheinz force-pushed the feature/allow-random-element-from-enum branch from 5460815 to 021e8d3 Compare May 2, 2023 08:09
@localheinz localheinz merged commit 096f7ff into FakerPHP:main May 14, 2023
@localheinz
Copy link
Copy Markdown
Member

localheinz commented May 14, 2023

Thank you, @cosmastech!

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

Reviewers

@localheinz localheinz localheinz approved these changes

@pimjansen pimjansen Awaiting requested review from pimjansen

@bram-pkg bram-pkg Awaiting requested review from bram-pkg

Assignees

@pimjansen pimjansen

@bram-pkg bram-pkg

Labels

enhancement New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@cosmastech @localheinz @pimjansen @bram-pkg