Variable naming #2945
Labels
meta-discussion
Discussion about the project itself, direction, (code) design, etc
Comments
|
Do you plan to fix that automatically or manually? |
|
I vote for using C for structs, no prefix for enums and the We should also use CamelCase for class and method naming, some code violates this rule such as ddnet/src/engine/client/graphics_threaded.cpp Line 117 in 113ec25 ddnet/src/engine/client/backend_sdl.cpp Line 552 in 113ec25 |
Automatically I'd guess. He has a script that extracts all identifiers using libclang and I have one that checks them for naming issues. We can run both on CI to ensure we don't commit any more naming mistakes. @heinrich5991 Huh, 1500 sounds like quite a lot. When we first ran it it was more like 100. I had issues with stuff like unions members before, did you fix those? I would allow For enums do you mean the constants or the enums themselves. We don't seem to have many tagged enums in the codebase. In anycase
I think we should fix it all in one go like we did with style. |
|
imo S for structs makes sense, bcs a struct is often just used as a bunch of data(maybe smaller member functions), while a class is something superrior( i know its basically the same in c++ ), but i'd use them like this. E for enum sounds good too xd |
For classes, functions and methods we should be using PascalCase. We don't use camelCase anywhere in the code iirc. |
yeah i meant that, just didnt remember the name, thats why i named it CamelCase ... |
Definitely in one go. |
The enums themselves. Enum constants are SCREAMING_SNAKE_CASE. |
|
This script will also transform all |
No, just allows |
|
What about |
|
1454 lines. Would fix them if you think this is an adequate amount. |
|
Fix them how? Is this implemented as a clang-tidy check? Haven't seen the source code yet. |
That looks wrong |
I guess we need a way to exclude iterators |
|
I think you really need to find your own code style, the code reads as if it was written by 200 people who do not follow the style of your project. I'm writing on behalf of a newcomer who just started to get into your code and it's hard to do. So too GetFTiles, the letter F magic apparently The style of the variables names I'd rather not say... I understood everything at once when I saw my friends in pairs. CCharacter *OwnerChar
CCharacter *pOwnerChar |
|
@kurosio I understand your point, but you have to remember there is alot of code that is already 10 years old. During that time alot happened :D |
|
I don't see the point of this comment. As you can see we are very well aware that the code style is very inconsistent and are working on getting it better.
A month or so ago we adopted clang-format, which made the style itself consistent.
A week or so ago we started using clang-tidy to catch bugs and modernize
the code.
Now this issue is about fixing any inconsistent naming.
From what I see we are on track to getting the code nice and neat.
…On Sat, Nov 14, 2020, 14:21 kurosio ***@***.***> wrote:
I think you really need to find your own code style, the code reads as if
it was written by 200 people who do not follow the style of your project.
I'm writing on behalf of a newcomer who just started to get into who
started to into your code and it's hard to do.
I see your default constructor that you offer me to work with functions
that have const specifiers, but why? to protect it when somewhere else you
let me know that I can change it freely.
So too GetFTiles, the letter F magic apparently
The style of the variables names I'd rather not say... I understood
everything at once when I saw my friends in pairs.
CCharacter *OwnerChar
CCharacter *pOwnerChar
|
|
I hope that someday the code can be read in the same style. With this comment I do not only mean |
|
@kurosio well then i can just say what @Learath2 said. We have this issue bcs we want to fix it. Also imo its not toooo horrible to read code with "wrong" code style, editors hightlight it with colors so it should be mostly clear what is meant |
7 tasks
|
Up? |
7 tasks
Robyt3
added
the
meta-discussion
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
The meta journey continues.
Proposed business rules about variable naming:
Classes, structs
Enum constants
Must be all uppercase letters, digits and underscores
Parameter/variable/member names
static const, member or not member may follow the Enum constant naming scheme.Example:
char **ms_ppArgs:ms,pp,Args.Unresolved questions
Other suggestions?
Should I fix all problem instances in one go, or shall we update them as we go? (EDIT: I currently count about 1500 instances, should be the right order of magnitude even if we change the rules slightly).
The text was updated successfully, but these errors were encountered: