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
Respell Move::None and Move::Null to reflect that they're constants #5206
base: master
Are you sure you want to change the base?
Conversation
…not functions Despite the syntax, these are compiletime singleton constants, or rather they *would* be if such syntax was permitted: ``` static constexpr Move None = Move(0); static constexpr Move Null = Move(65); ``` ...but this syntax not permitted. In some ways this is a flaw of the language, but that's offtopic (and a gigantic can of worms best left sealed). So function syntax is what we're stuck with, but we can respell them to reflect that they really are just constants. Also move MoveType under its explanatory comments and remove some blank lines No functional change
clang-format 17 needs to be run on this PR. (execution 8909920007 / attempt 1) |
I don't think this is a good enough reason to deviate from our usual convention on function names. |
These aren't function names tho. |
I haven't seen using a different convention for method names only because they represent constants... |
I guess the unconventional part is to represent constants as functions to begin with in such a not so "functional programming language" but what do we know.. |
How the heck often do you see methods representing constants????? That's a Bad Idea™ imo, an anti-pattern to be avoided. The only reason it occurs here is because of C++ limitations (can of worms etc etc) |
It has nothing to do with limitations, it can be easily done in the same soul the refactor did.. |
As I told Disservin on Discord, I do think there is much value to having these singleton constants in the |
…not functions
Despite the syntax, these are compiletime singleton constants, or rather they would be if such syntax was permitted:
...but this syntax not permitted. In some ways this is a flaw of the language, but that's offtopic (and a gigantic can of worms best left sealed). So function syntax is what we're stuck with, but we can respell them to reflect that they really are just constants.
Also move MoveType under its explanatory comments and remove some blank lines
No functional change