Inconsistent / Poor code style

[marcuscastelo]

TL;DR

Code style is currently inconsistent, and some choices are questionable (e.g. functions names).
I think we should stablish a fixed code style to avoid messy code in the future.
Feel free to give suggestions in the comments.

Detailed explanation

I know code style is something that really depends on the team's / person's preferences, but there are a lot of lines that most people would agree are not part of any code style.

Here are some lines of code (of many), just to exemplify what I'm trying to say:

Mix of Spaces and Tabs (not visible on github code preview)

Inconsistent use of curly brackets

Right after function declaration:

snaskii21/src/ttsnake.c

Lines 292 to 294 in 33eb585

	void draw_settings(scene_t *scene){
	char buffer[NCOLS];

On the next line:

snaskii21/src/ttsnake.c

Lines 307 to 311 in 33eb585

	void playgame (scene_t* scene)
	{
	struct timespec how_long;

Inconsistent positioning of the * character on pointers

snaskii21/src/ttsnake.c

Lines 335 to 337 in 33eb585

	void * userinput()
	{

snaskii21/src/ttsnake.c

Lines 359 to 361 in 33eb585

	int main(int argc, char **argv)
	{

snaskii21/src/ttsnake.c

Line 375 in 33eb585

char *curr_data_dir = (char *)malloc((strlen(DATADIR "/" ALT_SHORT_NAME) + 1) * sizeof(char));

snaskii21/src/ttsnake.c

Line 228 in 33eb585

void showscene (scene_t* scene, int number, int menu)

snaskii21/src/ttsnake.c

Line 123 in 33eb585

int readscenes (char *dir, char *data_dir, scene_t** scene, int nscenes)

Proposed standard coding style

Based on the majority of lines already written, I've observed the following pattern:

• variables are named in snake_case
• functions nouns are not separated at all, so thislonglongfunctionwouldbesomethinglikethat();
• constants are ALL_CAPS_WITH_UNDERSCORE
• macros are the same as functions (e.g. sysfatal)
• structs are snake_case like variables, but its typedef is snake_case_t (_t at the end)
• most of the identation is set to use only SPACES (2 spaces) (not tabs).
• most of code blocks' open curly bracket starts on the next line
• pointer asterisk positioning doesn't seem to have any pattern

Here is my opinion on those topics:

As for functions and macros being everythnigtogheterprettyunreadable, I really think it's a bad idea to follow this code style, although I see where this came from (ncurses functions use this style). I also think mixing pointer asterisk placement can be confusing for some people, so I would suggest sticking to only one in all scenarios.

All the other stuff I've listed before are pretty standard, but are not consistent throughout the code.

For the pointer placement, my personal preference is to use type *pointer1, *pointer2; for declarations of pointers, void *func(type *pointer);

For function names, maybe we should use snake_case, just like pthread_create.

snaskii21/src/ttsnake.c

Line 459 in 33eb585

rs = pthread_create (&pthread, NULL, userinput, NULL);

Variables can still be snake_case as well, but I like to change them to cammelCase in my codes to differentiate them (just personal preference).

I think this needs discussion, and I want to emphasize I am not imposing any code style and I'm opening this issue so that we can discuss a code style to stick to throughout the development of this project.

[math-araujo]

Hi! I agree that this is a important issue. I agree with the code style you proposed on most parts.

For the pointer placement, my personal preference is to use type *pointer1, *pointer2; for declarations of pointers, void *func(type *pointer);

Personally, I agree with this answer on SO. I think it's easier to read pointer variables by declaring each one in a separate line. What do you think?

Variables can still be snake_case as well, but I like to change them to cammelCase in my codes to differentiate them (just personal preference).

I prefer snake_case for variables, but since this is personal, let's see what others think!

Thanks for opening this issue, it's a really needed one :)

[Float07]

I think The most important thing should be consistency. That said,

Personally, I agree with this answer on SO. I think it's easier to read pointer variables by declaring each one in a separate line. What do you think?

I agree with this. I think one declaration per line should be used since it makes the code more readable. As for the asterisk placement I don't have a preference about that.

I normally use camelCase for variables and PascalCase for structs and typedefs, but this is just my preference and, as I said before, the most important thing is keeping the consistency about this throughout the code.

[monacofj]

Good point. I agree with the pro-consistency argument (original code received contributions from varying sources, with no agreed upon conventions).

If I were to opine, and being that a standard C program, I would go for the tradition (lowercase identifiers etc.). Most important, we should choose if the program will be strict ansi (c90), or else c99 compatible (this will affect portability). It is already POSIX, though (what about cross compiling to Win32, eventuall?)

I am perhaps among the few old-school nerds who likes GNU coding style [1], and that's why the example comes with all-caps macro constants, lowercase function-like macros, other-line curly braces, Emacs-like indentation, some weird spacing between parenthesis and so on. I know it's a quasi-religious field but I'll do my best to hear the community (might I have at least the next-line block opening?) It has also some suggestion for source comments.

That said, I like void *p, *q (because we can have two variables here, in the same of separate lines) and endorse int my_underscored_function() with meaningful, yet not too long, names. I also appreciate gcc's -Wall and --Wextra suggestions, including about ambiguous parenthesis, unused declarations etc. I thing this helps to keep the source tidy, easy to read, and warning-free for a variety of compilers.

[1] GNU Coding Standards

[math-araujo]

I am perhaps among the few old-school nerds who likes GNU coding style [1], and that's why the example comes with all-caps macro constants, lowercase function-like macros, other-line curly braces, Emacs-like indentation, some weird spacing between parenthesis and so on. I know it's a quasi-religious field but I'll do my best to hear the community (might I have at least the next-line block opening?) It has also some suggestion for source comments.

Next-link block opening is the Allman style? If so, I completely agree! Furthermore, I would suggest to always use braces in compound statements, even for one-line statements. That is, I would suggest that we use

if (condition)
{
    foo();
}

instead of

if (condition)
    foo();

IMO it's easy to make a mistake in the second version, if it happens that we need to add something to the if-block afterwards.

[lucasvianav]

Great issue!

I agree that consistency is important, but personally I'd prefer:

• snake_case in everything and UPPER_CASE_SNAKE_CASE for constant
• Trailing _t for structs
• Indentation with spaces (either 2 or 4 is fine by me)
• Opening curly bracket on the same line as the statement with spaces around the parenthesis for function declarations (int foo(args) { ...) and all around for if-like statements (if (condition) { ... (sorry bout that, @monacofj). Aside from personal preference, I think in general it makes it easier to read/understand the code.
• Pointer asterisk next to the variable name rather than the type (int *a instead of int* a). I'm okay with multiple pointers in the same line if the asterisk is like that (int *a, b, *c is fine but int* a, b,* c is not), but don't mind otherwise.

Furthermore, I would suggest to always use braces in compound statements, even for one-line statements.

I'm 200% for that, although I think if (condition) { foo(); } is also fine

[lucasvianav]

Maybe we should pin this issue?

[Float07]

I don't have a really strong personal preference about the subject so I think I won't give suggestion about the choice we should make. But maybe we should create a document with the standard we set?

Maybe we should pin this issue?

I think so, since we cannot code if we don't set how we should code. I think we also need to make this issue critical, because it needs to be solved ASAP if we want to have something done.

[math-araujo]

• Opening curly bracket on the same line as the statement with spaces around the parenthesis for function declarations (int foo(args) { ...) and all around for if-like statements (if (condition) { ... (sorry bout that, @monacofj). Aside from personal preference, I think in general it makes it easier to read/understand the code.

I disagree, I'll side with @monacofj on this one :P I think it's worse for read/understand the code.

• Pointer asterisk next to the variable name rather than the type (int *a instead of int* a). I'm okay with multiple pointers in the same line if the asterisk is like that (int *a, b, *c is fine but int* a, b,* c is not), but don't mind otherwise.

I agree about asterisk placement (int *a instead of int* a), since this seem to be a more common convention in C (in C++ it's reverse). Disagree about multiple variables declared on the same line: I vote for one variable per line.

I'm 200% for that, although I think if (condition) { foo(); } is also fine

I disagree about keeping the block on the same line as the condition, even for one-liners; it's harder to maintain IMO. I would suggest to use

if (condition)
{
    foo();
}

or at least (if the K&R Style bracing style is adopted):

if (condition) {
    foo();
}

[LeoFonsecaP]

• Opening curly bracket on the same line as the statement with spaces around the parenthesis for function declarations (int foo(args) { ...) and all around for if-like statements (if (condition) { ... (sorry bout that, @monacofj). Aside from personal preference, I think in general it makes it easier to read/understand the code.

I disagree, I'll side with @monacofj on this one :P I think it's worse for read/understand the code.

I agree with @Float07. This is a topic I have a strong opinion on, I feel like next line opening curly brackets are confusing and I would much rather use same liners.

• Pointer asterisk next to the variable name rather than the type (int *a instead of int* a). I'm okay with multiple pointers in the same line if the asterisk is like that (int *a, b, *c is fine but int* a, b,* c is not), but don't mind otherwise.

I agree about asterisk placement (int *a instead of int* a), since this seem to be a more common convention in C (in C++ it's reverse). Disagree about multiple variables declared on the same line: I vote for one variable per line.

Neither here nor there on this one, I´ll go with what makes more sense to everyone in regards to how many variables per line. I usually use type* var, but would not mind to switch for the project.

I'm 200% for that, although I think if (condition) { foo(); } is also fine

I disagree about keeping the block on the same line as the condition, even for one-liners; it's harder to maintain IMO. I would suggest to use

if (condition)
{
    foo();
}

or at least (if the K&R Style bracing style is adopted):

if (condition) {
    foo();
}

I agree with @math-araujo on this one, not a fan of one line conditions, I would stick with:
if (condition) {
foo();
}

Finally, when it comes to indentation, I believe one tab would be the best way to do it. It´s easier to do and I like the space it provides.

[lucasvianav]

We should also set a max width for the lines. I like 80 characters

[monacofj]

Oh boy, this will probably be the longer thread in the whole project.... and likely never resolved, rs.

[lucasvianav]

Oh boy, this will probably be the longer thread in the whole project.... and likely never resolved, rs.

Yeah, it's probably worth it to make a poll to decide on each item? Don't really think we're gonna be able to convince each other on the subject hahaha

[LeoFonsecaP]

Yeah, it's probably worth it to make a poll to decide on each item? Don't really think we're gonna be able to convince each other on the subject hahaha

I'm down for a poll. Can we run one in here?

[lucasvianav]

I'm down for a poll. Can we run one in here?

We can with this app. I can add it to the repo and create one if everybody is ok with that. What are all the options we have to vote?

[lucasvianav]

So, I think our options are as follow. Am I forgetting something? If y'all agree I can create a poll.

Variables/functions formatting

• snake_case
• camelCase

Structs formatting

• snake_case_t
• PascalCase

Indentation

• Tabs
• 2 Spaces
• 4 Spaces

Blocks

• Same-line curly brackets separated by spaces (if (condition) { ...)
• Same-line curly brackets without spaces (if (condition){ ...)
• Next-line curly brackets

If-like statements

• Spaces before parenthesis (if (condition))
• No spaces before parenthesis (if(condition))

Pointer asterisks

• int *a
• int * a
• int* a

Declare multiple variable in same line

• Yes
• No
• Only if there are no pointers
• Only if they aren't all pointers
• Only if they are all pointers

One-liners

• Get rid of the brackets (please don't): if (condition) variable = value;
• Keep it in one line but with brackets: if (condition) { variable = value; }
• Break to multiple lines nevertheless:

if (condition) {
  variable = value;
}

Line's max-width

• 80
• ???

[gsasouza]

After deciding which style to follow, we can use some auto-formatter tool like Clang-Formatter with a pre-commit hook to ensure that everyone will follow with no effort needed

[lucasvianav]

There we go. I don't think I can close the polls, so we should set a deadline for voting. What do you all think?

Unfortunately, if you're using a dark theme for GitHub you may have to disable it to better read the poll's options (they're incompatible and I have no control over it). You can do that in settings > appearance.

Also, this comment has some examples for the options.

Variables/functions formatting

Structs formatting

Indentation

Blocks

If-like statements

Pointer asterisks

Declare multiple variable in same line

One-liners

Line's max-width