General Category > Implementation Details

Code formatting of C2's c++

(1/2) > >>

lerno:
This might seem like a minor issue, but wading through thousands of lines of Clang (I've cut away in excess of 30,000 lines of code), there are two coding habits that makes the code IMMENSELY hard to read.

One is single line if's:


--- Code: ---if (foo)
  do_something();

--- End code ---

This obviously is even worse when adding else as well. The problem here is when you try get an overview of  a function. Here K&R + single statement on two lines is making something really horrid:


--- Code: ---namspace foo {

/* many lines of code */

  void a_method() {
  /* MANY lines of code */
    do_struff();
    if (foo)
      do_something();
  } // <- What ends here??

/* more lines of code */

} // <- What ends here???

--- End code ---

Basically it's hard what block ends where.

I prefer (1) single statements ifs ALWAYS on a single line, Allman style. Any of those would solve the problem, BOTH is best:

ALLMAN

--- Code: ---namspace foo
{

/* many lines of code */

  void a_method()
  {
  /* MANY lines of code */
    do_struff();
    if (foo)
      do_something();
  }

/* more lines of code */

}

--- End code ---


K&R Single line:

--- Code: ---namspace foo {

/* many lines of code */

  void a_method() {
  /* MANY lines of code */
    do_struff();
    if (foo) do_something();
  }

/* more lines of code */

}

--- End code ---

ALLMAN single line:

--- Code: ---namspace foo
{

/* many lines of code */

  void a_method()
  {
  /* MANY lines of code */
    do_struff();
    if (foo) do_something();
  }

/* more lines of code */

}

--- End code ---

I understand most have a preference for K&R, so I'm not going to fight for Allman. But that two line single statement is a killer.

Switch-case where "case" does not have indention is difficult for similar reasons, although the two line if is by far the worst.

bas:
I played around with clang-format and that seems to be a good candidate. I'll look at an initial style and propose that..

There is a .clang-format file in the root of the archive. It can be applied by running

--- Code: ---clang-format-i main.cpp
--- End code ---

Please use that on several files and check whether it does something really nasty or bad style-wise

lerno:
It looks like spaces went from 3 -> 4?  :o

lerno:
We can certainly use 4, but I'd just mention that this is different from how the current code looks that uses 4.

- Row breaks comments. I don't know if this is a good idea.

- Is it possible to require { } on if-else where each is a single statement?

Eg:


--- Code: ---if (Res.isUsable()) Cases.push_back(Res.get());
else return StmtError();

// =>

if (Res.isUsable()) {
  Cases.push_back(Res.get());
} else {
  return StmtError();
}

--- End code ---

Note that single line ifs are fine! Just not anything multi-line should have { }

Sometimes linebreak is weird:


--- Code: ---    StmtResult Res = Actions.ActOnDeclaration(id->getNameStart(), idLoc, type.get(), InitValue.get());
// becomes
   StmtResult Res =
       Actions.ActOnDeclaration(id->getNameStart(), idLoc, type.get(), InitValue.get());
// instead of
    StmtResult Res = Actions.ActOnDeclaration(id->getNameStart(), idLoc,
                                              type.get(), InitValue.get());

--- End code ---

bas:
For if() statements I always follow the Linux kernel syntax:
- allow simple if statement

--- Code: ---if (x) func();
--- End code ---

- allow single if() statement followed by simple else

--- Code: ---if (x) func1();
else func2();

--- End code ---

- if either the if or the else part needs brackets, both should have them.

so this is NOT allowed:

--- Code: ---if (x) func1();
else {
  //...
}

--- End code ---

Otherwise it depends on the code whether you want to use brackets or not for simple if's

Navigation

[0] Message Index

[#] Next page