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

Enum support for ArrayHelper::toArray() #20065

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

uaoleg
Copy link
Contributor

@uaoleg uaoleg commented Nov 6, 2023

Q A
Is bugfix? ✔️
New feature?
enum MessengerEnum: string
{
    case WhatsApp = 'whatsapp';
    case Viber    = 'viber';
    case Telegram = 'telegram';
}

Current output of ArrayHelper::toArray is:

array(3) { [0]=> array(2) { ["name"]=> string(8) "WhatsApp" ["value"]=> string(8) "whatsapp" } [1]=> array(2) { ["name"]=> string(5) "Viber" ["value"]=> string(5) "viber" } [2]=> array(2) { ["name"]=> string(8) "Telegram" ["value"]=> string(8) "telegram" } }

Proper way:

array(3) { [0]=> string(8) "whatsapp" [1]=> string(5) "viber" [2]=> string(8) "telegram" }

Copy link

what-the-diff bot commented Nov 6, 2023

PR Summary

  • Modification to toArray Method in BaseArrayHelper.php
    Changes were made in the toArray method inside the BaseArrayHelper.php file. There's an update in the condition check to investigate if a certain variable ($value) is a part of a special data type known as \UnitEnum. If found true, this variable's value gets placed into another object ($object[$key]). This can potentially enhance the control and handling of particular entities in our system.

Copy link

codecov bot commented Nov 6, 2023

Codecov Report

Attention: Patch coverage is 66.66667% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 64.82%. Comparing base (bf3ada1) to head (6157305).

Files Patch % Lines
framework/helpers/BaseArrayHelper.php 66.66% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master   #20065      +/-   ##
============================================
- Coverage     64.82%   64.82%   -0.01%     
- Complexity    11378    11379       +1     
============================================
  Files           429      429              
  Lines         37077    37079       +2     
============================================
+ Hits          24037    24038       +1     
- Misses        13040    13041       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@rhertogh
Copy link
Contributor

rhertogh commented Nov 6, 2023

I think that converting an enum to an array of values makes indeed much more sense than the default behavior.
But in case of a backed enum I would expect an associative array:

array(3) {
  ["WhatsApp"]=>
  string(8) "whatsapp"
  ["Viber"]=>
  string(5) "viber"
  ["Telegram"]=>
  string(8) "telegram"
}

@bizley
Copy link
Member

bizley commented Apr 6, 2024

Please remember that only BackedEnum has "value", UnitEnum has only "name".

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.

4 participants