[Spice-devel] Coding style and naming conventions for C++

[Christophe de Dinechin]

Hi Lukáš,


In the specific case of the streaming agent, I believe it matters
for instant productivity that the code follow a style that does not
require additional thinking on Frediano’s part. So if Frediano likes
it, it’s fine by me, otherwise don’t care.

Also, rather than invent a style, I’d rather adopt an standard coding
style, e.g. Google’s. And then use clang-format to enforce all the
machine-enforceable parts of it.


Regards,
Christophe

PS: Some comments on your suggestions anyway…

> On 29 Jan 2018, at 15:19, Lukáš Hrázký <lhrazky at redhat.com> wrote:
> 
> Hello everybody,
> 
> I'd like to discuss a few things about the coding style for C++ in
> Spice (looking at the streaming agent atm).
> 
> Trying to keep this short and concise.
> 
> 
> 1. Method names
> Currently the method names are in CamelCase throughout the streaming
> agent. Methods are basically functions attached to a class, I suggest
> we use snake_case to be consistent with the function names.
> 
> It's rather confusing when you see a call like SomeObject(), which
> looks like a constructor, but you actually find out it's a method call
> from another method of the same class.

Naming a method with a name that can also be a class is always
ambiguous, CamelCase or not. Is color() a method or an ctor?
So DeCamelCaseIfication not a solution to that problem.

BTW, CamelCase is so frequent in C++ that it often can be used to identify
code as being C++ as opposed to plain C. To wit: LLVM, WebKit, Qt, etc.

> 
> 
> ;2. Namespace names
> Although not standard (you may have different experience), usually
> namespaces are lowercase in C++.

By that token, so do classes (in all of the standard library).
But it’s generally not true outside of the standard library.

> Also, they are hierarchical, I suggest
> we use that and in streaming agent we change the namespace like so:
> 
> SpiceStreamingAgent -> spice::streamingagent
> 
> or (imho better):
> SpiceStreamingAgent -> spice::streaming_agent
> 
> And stick to this scheme, i.e. lowercase and toplevel namespace
> 'spice', inside it a namespace of the component.

Not against the idea, but two levels of namespace for
2000 LOCs seems a tad bit overkill…


> 
> 
> 3. Namespace coding style
> 
> a) Let's not use `using namespace ...` ever even in .cpp files (see
> i.e. [1]). In streaming agent we have at the beginning of every .cpp:
> 
> using namespace std;
> using namespace SpiceStreamingAgent;

Again, 2000 lines of code, unlikely to grow much.
Google’s rule applies to their mega-projects, but for the agent,
I think that “using namespace” makes the code leaner.

> 
> For namespace std, "std::" is not a long prefix, clearly expresses the
> identifier is from the standard library and AFAIK most C++ projects use
> it this way.
> 
> For namespace SpiceStreamingAgent, I didn't even know it worked for
> definition of symbols in the namespace. First time I see it, it is very
> unusual. see b).
> 
> b) Let's keep the following coding style for namespaces, i.e. for
> streaming agent:
> 
> namespace spice {
> namespace streaming_agent {
> 
> THE_CODE
> 
> }} // namespace spice::streaming_agent

Not too enthusiastic about }}

> 
> 
> We should add the guidelines to the website next to the C coding style,
> but I have no intention to be exhaustive (see [1] for how long it can
> be), let's add important cases as they come up and just use common
> sense, keep the style of the local code and codereview to keep things
> in check?

Let’s first share our preference on existing styles to see if we agree on anything…
As for me, I have a slight preference for the LLVM coding style, but I made
modifications in my own clang-format files.


Regards
Christophe


> 
> Lukas
> 
> 
> [1] https://google.github.io/styleguide/cppguide.html#Namespaces
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel