on coding style

I vastly prefer coding to discussing coding style, just as I would prefer to write poetry instead of talking about how it should be written. Sometimes the topic cannot be put off, either because some individual coder is messing up a shared code base and needs to be corrected, or (worse) because some officious soul has decided, "what we really need around here are some strongly enforced style rules!" Neither is the case at the moment, and yet I will venture a post on the subject. The following are not rules, but suggested etiquette. The idea is to allow a coherent style of coding to flourish safely and sanely, as a humane, inductive, social process.

Maxim M1: Observe, respect, and imitate the largest-scale precedents available. (Preserve styles of whitespace, capitalization, punctuation, abbreviation, name choice, code block size, factorization, type of comments, class organization, file naming, etc., etc., etc.)

Maxim M2: Don't add weight to small-scale variations. (Realize that Maxim M1 has been broken many times, but don't take that as license to create further irregularities.)

Maxim M3: Listen to and rely on your reviewers to help you perceive your own coding quirks. (When you review, help the coder do this.)

Maxim M4: When you touch some code, try to leave it more readable than you found it. (When you review such changes, thank the coder for the cleanup. When you plan changes, plan for cleanups.)

On the Hotspot project, which is almost 1.5 decades old, we have often practiced and benefited from such etiquette. The process is, and should be, inductive, not prescriptive. An ounce of neighborliness is better than a pound of police-work.

Reality check: If you actually look at (or live in) the Hotspot code base, you will find we have accumulated many annoying irregularities in our source base. I suppose this is the normal condition of a lived-in space. Unless you want to spend all your time polishing and tidying, you can't live without some smudge and clutter, can you?

Final digression: Grammars and dictionaries and other prescriptive rule books are sometimes useful, but we humans learn and maintain our language by example not grammar. The same applies to style rules. Actually, I think the process of maintaining a clean and pleasant working code base is an instance of a community maintaining its common linguistic identity. BTW, I've been reading and listening to John McWhorter lately with great pleasure.

(If you end with a digression, is it a tail-digression?)

Comments:

It's amazing that we need to write rules for simple common sense, but unfortunately many people really need that :) I would add a final "maxim", critical for code reviewing: be aware of versioning. Don't reformat code that is not so bad (or just not in your own preferred style, or 100% adherent to the project standards if any); BUT if you need to do that, formatting/style/indentation/etc. changes of any significant size should be isolated, don't mix that with functional changes committed together in the same changeset.

Posted by Osvaldo Pinali Doederlein on December 01, 2011 at 01:04 AM PST #

Good point, Osvaldo. It's often unfair to ask a reviewer to review unrelated changes simultaneously, especially if they obscure each other.

Separating unrelated changes is not an absolute rule (in my group, at least). We encourage each other to incorporate cleanups in changesets, if the cleanups are somehow related to or close to the heart of the changeset. If I touch line 200 in a file, and line 205 has a typo in the comment, I will fix line 205 also, if it feels like a "light" change. That's part of the thinking behind M4: Leave the place nicer than you found it.

Bundling minor changes together can be abused, of course. We rely on the common sense of the group to notice when someone is reaching too far. Reviews are great!

Posted by John Rose on December 01, 2011 at 01:00 PM PST #

The only way I have found to get a fully consistent code style is to actually enforce it with a pre-commit hook in the version control system. This works very well if you can start using it when the project is young, trying to add it to an existing code base tend to give too many warnings/bugs and stop work too much.

Most automatic checkers will still allow some wiggle room so common sense is still needed to keep the overall style good.

Posted by Robert Olofsson on December 02, 2011 at 04:51 AM PST #

One thing that really annoys me, though, is when indentation whitespace is a mixture of spaces and TABs to the extent that both may be present on the same line. Parts of a source file been indented using 4-space tab expansion, while other parts have been indented using 8-space tab expansion. This is evident in the core JSE libraries. It leaves the code difficult to read and being a "broken window" leads to further source formatting deterioration.

Another issue is mixing c-style and java-style braces in the same file.

Inconsistency is the killer here.

What should be done once M1..M4 have been ignored?

Posted by Morten Hattesen on December 07, 2011 at 05:01 AM PST #

Post a Comment:
  • HTML Syntax: NOT allowed
About

John R. Rose

Java maven, HotSpot developer, Mac user, Scheme refugee.

Once Sun and present Oracle engineer.

Search

Categories
Archives
« April 2014
SunMonTueWedThuFriSat
  
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
   
       
Today