Skip to content

Pascal Case for constants #67

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

Closed
Kocicak opened this issue May 18, 2022 · 11 comments
Closed

Pascal Case for constants #67

Kocicak opened this issue May 18, 2022 · 11 comments

Comments

@Kocicak
Copy link

Kocicak commented May 18, 2022

8bf7455

Hi, in version 3.1.3, constant NAMESPACE_SEPARATOR disappeared and is no longer available. And also backward compatible name has not been added. Can you please add upper snake case constant?

Also, why Pascal Case for class constants? This is opposed to PSR-12, which you claim, that Nette follows: https://doc.nette.org/en/contributing/coding-standard. PSR-12 states that PSR-1 must be adhered. And PSR-1 states that "Class constants MUST be declared in all upper case with underscore separators". So why is Nette not following PSR-12 coding standard?

EDIT: also, why is backward-incompatible change released as patch version and not major version?

Thanks.

@milo
Copy link
Member

milo commented May 18, 2022

It is marked as @internal.

@dg
Copy link
Member

dg commented May 18, 2022

The reason is that PascalCase constants look better in the code. I've fixed the documentation.

@dg dg closed this as completed May 18, 2022
@Kocicak
Copy link
Author

Kocicak commented May 18, 2022 via email

@petrparolek
Copy link

The reason is that PascalCase constants look better in the code. I've fixed the documentation.

zase nesmyslné novinky! Nechceš zavádět místo dlouhých názvů zkratky? :D Stále nechápu modu!Programování není o modních výstřelcích ale o standardech.

@dg
Copy link
Member

dg commented May 18, 2022

@Kocicak I won't miss a prick like you around here. Bye-bye.

@petrparolek Please use English

@Kocicak
Copy link
Author

Kocicak commented May 19, 2022

Ok, so to make the code more pretty, you now have all constants defined twice. Once with upper snake case, once with pascal case. Not to mention when using those constants, IDE will offer you every constant twice, in each case. I wonder which case will everyone following PSR-1 choose :)

Can't help it, but the code doesn't seem prettier or improved in any way.

PS: I wasn't saying I'm leaving, just it's a reason to. These kind of changes and your reasoning about them is not what one expects in mature, stable project. There was no discussion, no explanation, just change in new patch version.

@milo
Copy link
Member

milo commented May 19, 2022

you now have all constants defined twice

BC?

will offer you every constant twice

Improvable by @deprecated?

which case will everyone following PSR-1

Probably blindly the upper case? With a small prayer everytime?

Can't help it, but the code doesn't seem prettier or improved in any way.

Try to use it month or two. Personally, I would never came back. Good code is readable like story. Much better with pascal case constants. One can focus on logic, not syntax.

just change in new patch version

without BC break

@Kocicak
Copy link
Author

Kocicak commented May 19, 2022

Improvable by @deprecated?

IDE will still offer you two variants, one will be struck-through. If I want to comply with PSR-1 in my project, I will use deprecated ones and IDE will mark those usages as errors / warnings. Not a great idea.

Try to use it month or two. Personally, I would never came back. Good code is readable like story. Much better with pascal case constants. One can focus on logic, not syntax.

Well, code is not a story. Logic tells you that constants are upper case, so when you see it, you don't need to focus on syntax. I think this is highly subjective, but when I read code with pascal case constants, It seems like I'm looking at static method call. So now I have to focus on syntax to know that this is not static method call, but poorly named constant. The resulting effect is opposite, readability is actually worse.

without BC break

But there was a BC break before it got fixed, wasn't?

@dg
Copy link
Member

dg commented May 19, 2022

BTW Capitalised class constants are leaving PHP itself with the advent of enums https://www.php.net/manual/en/language.enumerations.basics.php

@mabar
Copy link

mabar commented May 19, 2022

If I want to comply with PSR-1 in my project, I will use deprecated ones

PSR-1 states that "Class constants MUST be declared in all upper case with underscore separators." It is about declaration, not about usage.

when I read code with pascal case constants, It seems like I'm looking at static method call

Then you will probably have the same problem with enum cases. Enums RFC uses PascalCase. e.g. Symfony and Monolog already do the same.

Also all methods should be camelCase anyway, according to PSR-1, don't they?

But there was a BC break before it got fixed, wasn't?

Anything marked @internal can be removed without notice.

@mabar
Copy link

mabar commented May 19, 2022

Oh, one more thing. Seems like the PSR-12 follow up may be okay with PascalCase too.

They state that: "This example encompasses some rules below as a quick overview" and then use use const Vendor\Package\{ConstantA, ConstantB, ConstantC}; in example.

They are now even discussing recommendation of PascalCase. (even though CamelCase is confusingly used in PR)

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

5 participants