Re: Coding Standard Discussion

Peter Kelly Tue, 24 Feb 2015 06:35:04 -0800

I’ll preface this with saying that I think that of all the tasks facing us in Corinthia, going through 54,000 lines of code (if you include the editor) and changing the coding style is probably the least important, and I would suggest that time could be used more productively on other improvements. That doesn’t mean we shouldn't change it - just something worth keeping in mind.

Anyway, here’s the rationale behind the style I used when writing the original code

100-length lines: There’s actually quite a few that are even longer than this; I was originally using 120. The reason for this was that the code was originally written in Objective C, which has *really* long method names, in particular because each parameter is named. Here’s just one example:

NSRange range = [str rangeOfString: @"[0-9]+" options: NSRegularExpressionSearch range: NSMakeRange(str,str.length)];

(and yes, it’s annoying - but Xcode’s autocompletion makes it ok once you get used to it).

After the transition to C, average line length was reduced, because of the simpler method call syntax. However, since C is not an object-oriented programming language, and we need to avoid name clashes between methods in the same “class", all of the functions for a particular “class” (data structure) are prefixed with the name, e.g.:

DFNode *abstract = WordConverterCreateAbstract(conv,HTML_BODY,concrete);

In C++, this would be:

DFNode *abstract = conv.createAbstract(HTML_BODY,concrete);

that is, the method would not be prefixed with the class name. Assuming class names are often in the range of around 10-15 characters, this results in more verbose code, and thus a longer required line length in order to express the same thing.

Another factor is that monitors are wider than they are taller. To maximise the amount of code visible on one screen at a time (assuming only a single file is visible at a time), longer lines result in greater visibility than shorter lines. A function that might be 30 lines of code might be 45 if the length reduced to 70 characters, and everything from assignment statements to function calls and declarations split over multiple lines because they don’t fit.

I would consider 80 characters to be the absolute minimum, if we were to change. But my preference is for 100 for the above reasons.

Spaces before function arguments: I agree this is a good idea

Brace styles: I haven’t looked up the exact K&R details or Gnu but again, for code density, I think having braces on the same lines is better. I haven’t done this for functions though - not for any particular reason, just out of habit.

Page breaks: I’m not sure what these mean in the context of text files.

For reference, I’ve attached a summary of the maximum line lengths in all files (and the script I used to generate it).

Having said all this, I really think we have far more important work to do - like, say, getting an editing UI built or adding support for ODF - I’ve kind of tuned out of the xmalloc discussion for the same reason.

63 ./DocFormats/DocFormats.c
63 ./DocFormats/api/headers/DocFormats/DFXMLForward.h
63 ./DocFormats/api/headers/DocFormats/DocFormats.h
63 ./DocFormats/api/headers/DocFormats/Formats.h
63 ./DocFormats/api/src/Formats.c
63 ./DocFormats/api/tests/APITests.c
63 ./DocFormats/core/src/common/DFClassNames.h
63 ./DocFormats/core/src/css/CSSSyntax.h
63 ./DocFormats/core/src/html/DFHTMLNormalization.h
63 ./DocFormats/core/src/lib/DFAllocator.h
63 ./DocFormats/core/src/lib/DFCharacterSet.c
63 ./DocFormats/core/src/lib/DFCharacterSet.h
63 ./DocFormats/core/tests/lib/LibTests.c
63 ./DocFormats/core/tests/xml/XMLTests.c
63 ./DocFormats/filters/latex/src/HTMLToLaTeX.h
63 ./DocFormats/filters/odf/src/ODF.c
63 ./DocFormats/filters/odf/src/ODF.h
63 ./DocFormats/filters/odf/tests/ODFTests.c
63 ./DocFormats/filters/ooxml/src/common/OOXMLTypedefs.h
63 ./DocFormats/filters/ooxml/src/word/WordWhitespace.h
63 ./DocFormats/filters/ooxml/src/word/lenses/WordField.h
63 ./DocFormats/headers/DFCore.h
63 ./consumers/dfutil/src/FunctionTests.h
63 ./consumers/dfutil/src/StringTests.h
64 ./DocFormats/core/src/xml/DFChanges.h
65 ./DocFormats/core/src/html/DFHTMLTables.h
65 ./DocFormats/core/tests/common/BDTTests.h
65 ./DocFormats/filters/odf/src/ODFPackage.h
65 ./DocFormats/filters/ooxml/src/word/WordSettings.h
68 ./DocFormats/core/src/lib/DFError.c
68 ./DocFormats/filters/latex/tests/LaTeXTests.c
70 ./DocFormats/api/src/Operations.c
70 ./DocFormats/filters/ooxml/src/word/WordStyles.h
71 ./DocFormats/headers/DFCommon.h
72 ./DocFormats/core/src/html/DFTidyHelper.h
72 ./DocFormats/core/tests/html/HTMLTests.c
74 ./DocFormats/core/src/lib/DFZipFile.h
75 ./DocFormats/core/src/lib/DFCallback.c
76 ./DocFormats/core/src/lib/DFCallback.h
76 ./DocFormats/filters/ooxml/src/word/WordWhitespace.c
78 ./DocFormats/core/src/html/DFTidyWrapper.h
80 ./DocFormats/core/src/names/DFXMLNames.c
80 ./DocFormats/core/src/names/DFXMLNames.h
80 ./DocFormats/core/src/names/DFXMLNamespaces.h
80 ./consumers/dfutil/src/Commands.h
81 ./DocFormats/filters/ooxml/src/word/CSSClassNames.h
81 ./DocFormats/filters/ooxml/src/word/formatting/WordNumPr.h
83 ./DocFormats/core/src/lib/DFAllocator.c
84 ./DocFormats/core/src/css/CSSProperties.c
85 ./DocFormats/core/src/lib/DFFilesystem.h
85 ./DocFormats/core/src/lib/DFString.h
85 ./DocFormats/filters/ooxml/tests/word/WordPlain.h
85 ./DocFormats/platform/tests/WrapperTests.c
86 ./DocFormats/core/src/html/DFHTML.h
86 ./DocFormats/core/src/lib/DFHashTable.h
86 ./consumers/dfconvert/src/main.c
87 ./DocFormats/filters/ooxml/tests/word/WordTests.c
88 ./DocFormats/filters/odf/src/ODFManifest.c
90 ./DocFormats/api/headers/DocFormats/DFError.h
90 ./DocFormats/api/headers/DocFormats/Operations.h
91 ./DocFormats/core/src/names/DFXMLNamespaces.c
94 ./DocFormats/filters/ooxml/src/word/formatting/WordCommonPr.h
94 ./DocFormats/headers/DFPlatform.h
94 ./DocFormats/headers/DFTypes.h
94 ./DocFormats/platform/src/Apple.c
95 ./DocFormats/filters/ooxml/src/word/WordSettings.c
95 ./build/CMakeFiles/3.0.2/CompilerIdC/CMakeCCompilerId.c
97 ./DocFormats/core/src/lib/DFFilesystem.c
97 ./DocFormats/core/src/xml/DFChanges.c
98 ./DocFormats/core/src/html/DFHTML.c
99 ./DocFormats/filters/ooxml/src/word/lenses/WordLenses.c
100 ./DocFormats/core/src/common/DFBDT.h
100 ./DocFormats/core/src/common/DFTable.c
100 ./DocFormats/core/src/common/DFTable.h
100 ./DocFormats/core/src/css/CSS.h
100 ./DocFormats/core/src/css/CSSLength.c
100 ./DocFormats/core/src/css/CSSLength.h
100 ./DocFormats/core/src/css/CSSParser.h
100 ./DocFormats/core/src/css/CSSProperties.h
100 ./DocFormats/core/src/css/CSSSelector.c
100 ./DocFormats/core/src/css/CSSSelector.h
100 ./DocFormats/core/src/html/DFHTDocument.h
100 ./DocFormats/core/src/lib/DFBuffer.c
100 ./DocFormats/core/src/lib/TextPackage.h
100 ./DocFormats/core/src/xml/DFNameMap.h
100 ./DocFormats/filters/odf/src/ODFManifest.h
100 ./DocFormats/filters/odf/src/ODFSheet.h
100 ./DocFormats/filters/ooxml/src/word/WordCaption.c
100 ./DocFormats/filters/ooxml/src/word/WordCaption.h
100 ./DocFormats/filters/ooxml/src/word/WordGC.c
100 ./DocFormats/filters/ooxml/src/word/WordGC.h
100 ./DocFormats/filters/ooxml/src/word/WordLists.h
100 ./DocFormats/filters/ooxml/src/word/WordPackage.h
100 ./DocFormats/filters/ooxml/src/word/WordSection.c
100 ./DocFormats/filters/ooxml/src/word/WordSection.h
100 ./DocFormats/filters/ooxml/src/word/WordTheme.c
100 ./DocFormats/filters/ooxml/src/word/WordTheme.h
100 ./DocFormats/filters/ooxml/src/word/formatting/WordCommonPr.c
100 ./DocFormats/filters/ooxml/src/word/lenses/WordBlockLevel.c
100 ./DocFormats/filters/ooxml/src/word/lenses/WordBody.c
100 ./DocFormats/filters/ooxml/src/word/lenses/WordBookmark.h
100 ./DocFormats/filters/ooxml/src/word/lenses/WordChange.c
100 ./DocFormats/filters/ooxml/src/word/lenses/WordDocument.c
100 ./DocFormats/filters/ooxml/src/word/lenses/WordDrawing.h
100 ./DocFormats/filters/ooxml/src/word/lenses/WordEquation.c
100 ./DocFormats/filters/ooxml/src/word/lenses/WordLenses.h
100 ./DocFormats/filters/ooxml/src/word/lenses/WordRunContent.c
100 ./DocFormats/filters/ooxml/src/word/lenses/WordSmartTag.c
100 ./consumers/dftest/src/main.c
100 ./consumers/dfutil/src/main.c
101 ./DocFormats/api/headers/DocFormats/DFStorage.h
101 ./DocFormats/core/src/css/CSSStyle.h
101 ./DocFormats/core/src/lib/DFBuffer.h
102 ./DocFormats/core/src/lib/DFZipFile.c
102 ./DocFormats/filters/ooxml/src/word/WordNotes.h
102 ./DocFormats/filters/ooxml/src/word/lenses/WordParagraphContent.c
103 ./DocFormats/core/src/css/CSSParser.c
103 ./DocFormats/core/src/html/DFTidyHelper.c
103 ./DocFormats/core/src/xml/DFDOM.h
103 ./DocFormats/platform/tests/OStests.c
105 ./DocFormats/core/src/xml/DFNameMap.c
105 ./DocFormats/core/tests/css/CSSTests.c
105 ./DocFormats/core/tests/html/HTMLPlain.h
105 ./consumers/dfutil/src/Commands.c
106 ./DocFormats/core/tests/html/HTMLPlain.c
106 ./DocFormats/filters/ooxml/src/word/lenses/WordTable.c
107 ./DocFormats/core/src/css/CSSSyntax.c
107 ./DocFormats/filters/ooxml/src/word/formatting/WordRPr.c
108 ./DocFormats/core/src/css/CSSStyle.c
108 ./DocFormats/filters/ooxml/src/word/Word.c
108 ./DocFormats/filters/ooxml/src/word/formatting/WordRPr.h
108 ./DocFormats/platform/src/Wrapper.c
109 ./DocFormats/core/src/css/CSS.c
109 ./DocFormats/core/src/html/DFTidyWrapper.c
109 ./DocFormats/filters/odf/src/ODFSheet.c
109 ./DocFormats/filters/ooxml/src/word/Word.h
109 ./DocFormats/filters/ooxml/src/word/WordNotes.c
109 ./DocFormats/filters/ooxml/src/word/lenses/WordHyperlink.c
109 ./DocFormats/platform/src/Linux.c
110 ./DocFormats/core/tests/common/BDTTests.c
110 ./DocFormats/filters/ooxml/src/word/lenses/WordField.c
110 ./DocFormats/filters/ooxml/src/word/lenses/WordRun.c
110 ./DocFormats/unittest/DFUnitTest.c
111 ./DocFormats/filters/odf/src/text/ODFText.c
111 ./DocFormats/filters/ooxml/src/word/lenses/WordParagraph.c
111 ./DocFormats/unittest/DFUnitTest.h
112 ./DocFormats/filters/odf/src/text/ODFText.h
113 ./DocFormats/core/src/lib/DFArray.c
113 ./DocFormats/core/src/lib/DFString.c
114 ./DocFormats/core/src/lib/DFArray.h
114 ./DocFormats/core/src/lib/DFStorage.c
114 ./DocFormats/core/src/xml/DFXML.h
115 ./DocFormats/core/src/html/DFHTDocument.c
115 ./DocFormats/filters/ooxml/src/word/lenses/WordDrawing.c
116 ./DocFormats/core/src/html/DFHTMLNormalization.c
116 ./DocFormats/core/src/xml/DFDOM.c
116 ./DocFormats/platform/src/Unix.c
116 ./DocFormats/platform/src/Win32.c
117 ./DocFormats/core/src/css/CSSSheet.c
117 ./DocFormats/core/src/xml/DFMarkupCompatibility.c
117 ./DocFormats/filters/latex/src/HTMLToLaTeX.c
117 ./DocFormats/filters/ooxml/src/common/OPC.c
117 ./DocFormats/filters/ooxml/src/common/OPC.h
118 ./DocFormats/core/src/common/DFBDT.c
118 ./DocFormats/core/src/css/CSSSheet.h
118 ./DocFormats/core/src/html/DFHTMLTables.c
118 ./DocFormats/core/src/lib/DFHashTable.c
118 ./DocFormats/core/src/xml/DFMarkupCompatibility.h
118 ./DocFormats/filters/ooxml/src/word/WordConverter.c
119 ./DocFormats/core/src/xml/DFXML.c
119 ./DocFormats/filters/ooxml/src/word/WordLists.c
119 ./DocFormats/filters/ooxml/src/word/WordNumbering.c
119 ./DocFormats/filters/ooxml/src/word/WordStyles.c
119 ./DocFormats/filters/ooxml/src/word/formatting/WordNumPr.c
120 ./DocFormats/core/src/lib/TextPackage.c
120 ./DocFormats/filters/ooxml/src/word/WordNumbering.h
121 ./DocFormats/filters/ooxml/src/word/WordObjects.c
121 ./DocFormats/filters/ooxml/src/word/lenses/WordBookmark.c
122 ./DocFormats/filters/ooxml/src/word/WordObjects.h
122 ./DocFormats/filters/ooxml/src/word/formatting/WordPPr.h
122 ./DocFormats/filters/ooxml/src/word/formatting/WordTblPr.c
124 ./DocFormats/filters/odf/src/ODFPackage.c
124 ./DocFormats/filters/ooxml/src/word/WordPackage.c
125 ./DocFormats/filters/ooxml/src/word/WordConverter.h
125 ./DocFormats/filters/ooxml/src/word/formatting/WordPPr.c
125 ./DocFormats/filters/ooxml/tests/word/WordPlain.c
126 ./DocFormats/filters/ooxml/src/word/WordSheet.c
126 ./DocFormats/filters/ooxml/src/word/WordSheet.h
130 ./DocFormats/filters/ooxml/src/word/formatting/WordTblPr.h
133 ./DocFormats/filters/ooxml/src/word/CSSClassNames.c

#!/usr/bin/env python3


filenames = []
with open('file-list') as f:
    filenames = filter(lambda x: x != '',f.read().split('\n'))

for filename in filenames:
    with open(filename) as f:
        lines = filter(lambda x: x != '',f.read().split('\n'))
    maxLen = 0
    for line in lines:
        if maxLen < len(line):
            maxLen = len(line)
    print(maxLen,filename)

—

Dr Peter M. Kelly

[email protected]

PGP key: http://www.kellypmk.net/pgp-key

(fingerprint 5435 6718 59F0 DD1F BFA0 5E46 2523 BAA1 44AE 2966)

On 24 Feb 2015, at 4:24 am, Gabriela Gibson <[email protected]> wrote:

Hi All,

Jan and Peter asked to start a discussion about coding standards on the
mailing list.(https://issues.apache.org/jira/browse/COR-46)

I think the Subversion project has a very good strategy, see here:
http://subversion.apache.org/docs/community-guide/conventions.html

Personally I think we could easily just copy their rules and will not
regret it, plus their rules have stood the test of time.

That all said, whatever rules are chosen I am fine with ;-)

Chars per line
--------------------

My personal rationale for liking 70 chars per line is that more gets
tedious to read (as in line too long) and, I like to be able to use emacs
over ssh, or, if local, -nw style.

Being a bit myopic (and a fan of my screen being at least 1m away from me),
I also manage to max out a 15" screen with just 70 chars, but my other half
(who is eagle eyed) manages to fit two editor windows next to each other
with 70 char wide each.

The other thing this does is introduce shape to the code, because long
lines now have to be broken up.

This help a lot when glancing over stuff on the quick, you see 3 lines and
know that this function declaration has 3 parameters, whereas if it's one
line, you have to count 'em. (and think! ;-) Plus, you can find stuff by
shape too, which can at times be useful, and if you have a long parameter
list, having the vars on their own line is also useful. (easy to count, and
easy to spot if they are in the wrong order for some strange reason and so
on).

So, 70 is quite a good number I think, plus it's traditional for most
software I've seen, and new people will find it easy to get used to our
code as well this way, since it'll looks instantly familiar and they won't
have to change their set ups (and habits) very much and find it easier to
switch between projects during the same day too, if they don't have to
remember greatly differing rules. (I swear that fingers have brains too).

Spaces before variables
---------------------------------

Regards spaces between variables --- I think it makes it easier to scan
quickly, that way you don't have to wonder, is this a full stop or a
comma? Also, it makes a parameter list read more like a sentence, making
the individual type or variable much easier to spot --- a long or short var
name is instantly recognisable, without having to read the entire thing.

Tabs/spaces
------------------

Everyone uses spaces, tabs usually is not popular, for a very good reason
which I forgot by now. Personally I like 8 spaces (which grosses everyone
out, heh) but 4 is a good (better!) number, it's separates code levels
nicely visually without making nested code too crowded. (Some people use 8
spaces to prevent code that is too deeply nested, notably, the Linux kernel
style guide does this.).

Brace styles
------------------

K&R or Gnu --- K&R has the advantage to preserving vertical screen real
estate, Gnu is easier to spot (you know where the brace sits and don't have
to scan to the end of the line, plus it's easier to copy an entire block).
I think both styles are equally fine, just not at the same time.

Page breaks
------------------

I find them handy if the file is large, but can live without them.

==============================================================================================

I also think that we should have the same discussion about log messages.
There is a lot to be said for uniformity (makes searches of the logs
visually much easier and also ensures that the logs are as informative as
can be) and I do like the svn rules here, and they also help a lot in
making patches higher quality and much easier to comprehend for readers.

Take a look:
http://subversion.apache.org/docs/community-guide/conventions.html#log-messages.

anyway, I hope you enjoyed reading this!

G

--
Visit my Coding Diary: http://gabriela-gibson.blogspot.com/

Re: Coding Standard Discussion

Reply via email to