C++ Programming Style

[Jorgen Grahn]

There is a super simple solution for handling formatting diffs:

1) submit a separate formatting change only revision prior to
making any substantive changes.

2) cmp the object files before and after the formatting only
changes to ensure no substantive changes were made

2a) if 2) fails explain and perform necessary verifications
and unit tests.

That's fine, but surely you see that it does not solve the problem I
outlined above?

The question is not "is this revision an indent-only change?" but
"what are the non-indent changes between now and then?". Some tools
let you ignore whitespace change, but that is not enough if you move
curly brackets around, etc.

This is just a special case of a more general guideline which is
to use the source control system in a modular way just as code is
best broken up into modules. In other words, don't submit single
massive everything-including-the-kitchen-sink revisions. Break up
changes and revisions into smaller sensible pieces.

I do that, but usually a mix of bad tool support and bad local
practices makes that fairly pointless. I produce clean and annotated
histories of my work -- which noone ever reads.

It would have been particularly useful as part of that reviewed
refactoring mentioned upthread. You can often convince yourself that a
single "transformation step" is harmless, but it's hard if you see
four or five lumped together.

/Jorgen

 

[Öö Tiib]

Noah suggested code review because I don't have access to a time
machine?

Yes. The earlier a defect is found the cheaper it is to fix (in terms
of time and effort). It goes more expensive a) in specification by
review b) by unit/automated testing, c) in code by review, d) by
testing and e) in hands of end user. So if you lack something in your
process that can discover defects earlier (and so make your software
development less time consuming) then you should add that into your
development process to achieve higher efficiency.

 

[Keith H Duggar]

That's fine, but surely you see that it does not solve the problem I
outlined above?

Having format-change-only revision points let's you (and this is
the key point) generate format-change-only /patches/. You can then
apply (using fuzzy/inexact patch application) these format-change-
only patches to prior revisions producing reformatted revisions for
comparison.

Sure, some hunks of a patch may fail due to conflicts but that is
no different from the "ordinary" conflicts you run into with, for
example, branch merges and can be resolved accordingly.

The question is not "is this revision an indent-only change?" but
"what are the non-indent changes between now and then?". Some tools
let you ignore whitespace change, but that is not enough if you move
curly brackets around, etc.

I understand. Hopefully the explanation above clears it up more.
The critical benefit of format-change-only revisions (as far as
the problem we are discussing) is that they enable generation of
format-change-only /patches/.

I do that, but usually a mix of bad tool support and bad local
practices makes that fairly pointless. I produce clean and annotated
histories of my work -- which noone ever reads.

It would have been particularly useful as part of that reviewed
refactoring mentioned upthread. You can often convince yourself that a
single "transformation step" is harmless, but it's hard if you see
four or five lumped together.

Yeah separating formatting changes into separate revisions will
not fix social problems nor every code comparison problem. But,
it does, in my experience, make comparison substantially easier
even in crappy situations where the last "working" revision was
from last year and 23 revisions ago.

KHD

 

[Noah Roberts]

33 AM, Jorgen Grahn wrote:
...

If it doesn't for you, either your brain is
differently wired[1], or the code you're reading is well-designed
with a low nesting level and short blocks.
...

And yes, we write well designed code. I don't know what to tell you if
you're skipping this step.

But I know what to tell *you*:

Less-than-perfect code is a fact of life, and our practices need to
take that into account. Most of the code we read is written by others,
and much of the time refactoring it is out of the question.

You might consider adding code reviews to your process. Sounds like
you've got some problems.

See above. How does code review help for legacy code, where
refactoring is out of the question? I don't have access to a time
machine ...

http://www.amazon.com/Working-Effectively-Legacy-Michael-Feathers/dp/0131177052

You might be right though. If you guys are having such difficulties
that *indentation style* makes your code difficult to read, hard to
review, and impossible to refactor... I don't know anything about that.

I do have experience with legacy code being difficult to refactor and
read...but it has absolutely nothing to do with how many spaces the
person used or where they put their braces. All of my problems with
legacy code can be boiled down to poor design choices and use of unsafe
language constructs. NEVER in my experience has formatting been the
major factor, or even A factor, in my ability to read code.

In fact, the only time I can think of that I've actually had to reformat
code in order to read it is when trying to debug template instantiation
errors the compiler vomits out. Even at that, it's a rare occurrence.

It seems to me that your team's problems go way beyond my experience so
maybe you're right....4 vs. 2 really DOES make a major difference in
such cases. On the other hand, you could very well be blaming the wrong
thing.

 

[Jorgen Grahn]

On 4/7/2011 12:51 AM, Jorgen Grahn wrote:
33 AM, Jorgen Grahn wrote:
...

If it doesn't for you, either your brain is
differently wired[1], or the code you're reading is well-designed
with a low nesting level and short blocks.
...

And yes, we write well designed code. I don't know what to tell you if
you're skipping this step.

But I know what to tell *you*:

Less-than-perfect code is a fact of life, and our practices need to
take that into account. Most of the code we read is written by others,
and much of the time refactoring it is out of the question.

You might consider adding code reviews to your process. Sounds like
you've got some problems.

See above. How does code review help for legacy code, where
refactoring is out of the question? I don't have access to a time
machine ...

[...]
It seems to me that your team's problems go way beyond my experience so
maybe you're right...

This thread is now old, but for the record, I don't think I wrote
about my team (current or previous ones) having any extraordinary
problems. Just less-than-perfect code -- in this case, too large
blocks and too deep nesting for me to be perfectly happy with a
two-space indent.

/Jorgen