Coding conventions

[M374LX]

https://github.com/lmms/lmms/wiki/Coding-conventions

The project's coding conventions have clearly changed many times since it started, while older code has not been updated. Therefore, we need to decide the definitive conventions and avoid changing them again.

I have already made some pull requests to update the conventions on the code, but a better idea is to decide the definitive conventions before continuing the updates.

One topic not covered in the wiki page is the naming of static class members. A possibility is the "s_" prefix, as currently used in some parts of the code.

It seems that the conventions for spaces in parentheses have changed, but the new conventions are not covered. Also, it is not defined whether there should be a space between keywords (if, while, for, switch, ...) and the following opening parenthesis. Many projects use such space.

Nothing is said about indenting statements spanning multiple lines.

Some topics in the page show example code with outdated conventions, like the topic about infix operators showing an example with snake_case, rather than camelCase.

One thing I do not agree to is the usage of the "true" and "false" keywords when comparing Boolean values. Instead of "if (x == true)" and "if (x == false)", I suggest to change it to simply "if (x)" and "if (!x)".

[tresf]

Alexandre, I think a good place to start is to help improve the wiki we have by making some edits and get some feedback.

I would recommend you do this on your branch and then post here for feedback.

@lukas-w has made most of the recent changes to our existing standards so I'd start with him. @diizy is good for feedback as well.

-Tres

[diizy]

The project's coding conventions have clearly changed many times since it started, while older code has not been updated. Therefore, we need to decide the definitive conventions and avoid changing them again.

The conventions have already been decided, they are listed on the coding conventions page. If something is not mentioned there, then it is up to each individual coder to decide. We don't need to be overly strict with the conventions because that'll just make things harder for everyone.

One topic not covered in the wiki page is the naming of static class members. A possibility is the "s_" prefix, as currently used in some parts of the code.

Static members are prefixed with s_ just like non-static members are prefixed with m_. This should be in conventions also.

It seems that the conventions for spaces in parentheses have changed, but the new conventions are not covered.

It's undefined so up to each coder to decide. Space usage is encouraged for complex nested parentheses, but not mandatory anymore.

[M374LX]

Here is the page: https://github.com/M374LX/lmms/wiki/Coding-conventions-%28update-suggestion%29

[tresf]

@M374LX, this looks pretty good. I'm prpbably not the right person to approve this so I'll wait for more feedback.

The one item I noticed is the use of outbuf (versus outBuf, etc)

[diizy]

We already have conventions and I don't see any reason to change them at the moment.

[tresf]

Vesa, from what I can see Alexandre's article is based off of ours, but tries to clear up a few areas that aren't clearly defined.

[lukas-w]

I agree with Vesa. Being too strict with the conventions and having too many rules just makes development harder for everyone. A good set of rules can be helpful in a large development team, which we don't have.

In general, I'd say less is more. I'd be fine with dropping most of our rules, maybe even all of them, because I don't think they are of any great use. Developers who care about a good coding style will be able look at the codebase and will try to make their code fit in and make it readable without reading any guidelines. Developers who don't care won't even bother reading the Coding conventions page. So who's left to read it? Time and effort may be spent more wisely on something else in the project.

One thing I do not agree to is the usage of the "true" and "false" keywords when comparing Boolean values. Instead of "if (x == true)" and "if (x == false)", I suggest to change it to simply "if (x)" and "if (!x)".

That's actually not suggested anywhere in our guidelines. ;) The paragraph you're probably referring to is about using true instead of e.g. TRUE.

[tresf]

Thanks for the feedback guys.

@M374LX the feedback from these two is about the best we're probably going to get, so we may want to consider closing this out as wont-fix.

-Tres

[M374LX]

Not a bad idea dropping most of the conventions, but the current conventions for naming (eg. uppercase initial for types, m_ prefix for member variables) really make the code easier to read. I still think all static member variables should be prefixed with s_.

[Wallacoloo]

Honestly, I think a more important thing than syntactic conventions to agree upon are appropriate language subsets.

For example, I tend to restrict myself to using the automatic memory management features of C++ (unique_ptr and the like) and avoid explicit delete when possible, on my own projects. I couldn't find any scoped pointers in the entire codebase though, so I've been assuming I should manually delete, since that's what all the rest of the code does.

Similarly, there are some one-time init functions that call new and use the returned memory up until program exit, but leave it to the OS to actually free their memory. I've also been assuming that those types of leaks are OK based on that. But I could be wrong about that, and maybe unique_ptrs should be favored over manual memory management - I wouldn't know without these conventions being documented.

[SecondFlight]

As part of a pruning effort, this enhancement request is archived into a dedicated "Better Workflow" checklist here #4877.

[tresf]

Specifically superceded by #4690. @M374LX, if the PR doesn't cover the boolean request, just request it in the PR. 👍