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

CamelCase vs UPPER_CASE #22

Closed
dereuromark opened this issue Jan 26, 2024 · 5 comments
Closed

CamelCase vs UPPER_CASE #22

dereuromark opened this issue Jan 26, 2024 · 5 comments

Comments

@dereuromark
Copy link

dereuromark commented Jan 26, 2024

Very interesting library!

One question:
Have you considered using what appears to be the convention for enums: CamelCase?

We had similar discussions for CakePHP for example:
cakephp/bake#972
And in the end seemed to go with those, as the cases are different from the constants in the end.
This way it also seems easier to distinguish the two.
It also seems to be the way they were intended to be in terms of convention by the core PHP (RFC).

Also saw a bit of both in your example so far: fc428d5#diff-b335630551682c19a781afebcf4d07bf978fb1f8ac04c6bf87428ed5106870f5R210

What do you think?

For form options we usually use

<option value="MultiWord">Multi Word</option>

as CamelCase for key vs Humanized Form for text.

Side note:
I also added a specific trait to mine for sorting and custom filtering (for some forms only a subset sometimes is wanted):
https://github.com/dereuromark/cakephp-tools/blob/master/src/Model/Entity/EnumTrait.php
Could be yours with a custom fn can also already do that out of the box, just doesnt mention it yet.

@stancl
Copy link
Member

stancl commented Jan 28, 2024

Hey, thanks for bringing this up.

I'd like to support all of these cases with sane defaults. So probably something like:

UPPERCASE_SNAKE_CASE => 'Uppercase snake case'
PascalCase => 'Pascal case'

The option values would remain the same as the case names (for pure enums, without a backing value) so that you can use fromName().

@stancl stancl closed this as completed in fea911e Jan 28, 2024
@stancl
Copy link
Member

stancl commented Jan 28, 2024

See the linked commit and let me know if that's how you'd intuitively expect it to behave (I included tests for both backed enums and pure enums). If it looks fine I'll tag a release, thanks!

@stancl
Copy link
Member

stancl commented Jan 28, 2024

Oops accidentally overrode the test for pure PascalCase enum when I was adding the test for the backed enum.

Fixed here 61ba793

@dereuromark
Copy link
Author

dereuromark commented Jan 28, 2024

I think my main point was that it could be useful to have
https://github.com/archtechx/enums?tab=readme-ov-file#usage
in line with the standard recommendation for it from PHP side using CamelCase
https://www.php.net/manual/en/language.types.enumerations.php

For people to use that mainly, and having upper case as optional thing, but not what usually would be used.

@stancl
Copy link
Member

stancl commented Jan 28, 2024

Ah that makes sense. I think with the changes I made here, all logic should be 1:1 consistent for both UPPER_SNAKE_CASE and PascalCase enum cases, so now the only remaining step would be to make the README consistent with the more standard syntax?

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

No branches or pull requests

2 participants