Re: Debian coding style?
In article <[🔎] 7gvft0$3rs$1@lina.lnx>,
Amy Fong <uwd38znr@umail.furryterror.org> wrote:
>These coding guidelines [...]
are Corel Linux's coding guidelines. I should point out that there are
at least _three_ such documents:
"Corel C++ Guidelines version [number] [date]"
"Corel Coding Standards version [number] [date]"
"Corel Linux C++ Coding Guidelines", a versionless, dateless,
authorless Wordperfect document.
For some reason, most of the Windows product developers seem to follow
the union of the first two documents, and it's only the unlucky souls
who are working on the Corel Linux distribution that deal with the third.
For brevity I'll refer to the first two as "Corel non-Linux" and the
third as "Corel Linux" guidelines.
The Corel non-Linux documents are slightly larger, and except for the
2-space tab stops, Hungarian notation, and Windows-specific stuff
irrelevant to Linux, they seem to be generally full of sound technical
ideas, or at least harmless ones. By contrast, at least half of the
"Corel Linux" guidelines, which only people who work on the Corel Linux
distribution seem to use, is anathema to most if not all open-source
projects and technical common sense.
No, I don't understand this either. Maybe Dilbert moved onto the 6th
floor somewhere. I guess I'm trying to have it stopped now.
The Corel Linux C++ Coding Guidelines is not a very easy document to
interpret in some places. Consider the statement:
>All new
>code written must follow these guidelines, including new functions added to
>existing sources files.
which is followed by the contradictory (or at least ambiguous):
>Modifications made to existing code may follow the
>coding style used by the original author in order to maintain readability.
Interpreting the latter statement alone means that the entire document
goes straight into the recycling bin, the madness ends, and we can all
just get on with our lives. If Debian and its component packages isn't
"existing code" I don't know what is. The problem of course is the
existence of the first statement, which seems to limit our ability to
apply the second.
Now here's where I come in:
>If you disagree with or don't understand the reasoning behind any of the
>following guidelines, you are encouraged to discuss your concerns with your
>project leader.
Well, I did--last week, I was dumb enough to write "[the Corel Linux
coding guidelines] appear to have been written by someone who has never
actually worked on a real Unix project in their life. [Compared to
Corel's coding guidelines, which are mostly harmless, Corel Linux's]
guidelines are downright _harmful_", and even worse, I wrote it in a
widely distributed internal memo.
Guess who `volunteered' to write Corel Wine coding style guidelines?
Just a couple of points I had already written that I haven't seen
mentioned here yet:
>c) Variable names must use hungarian notation (see below) followed by
>upper-case followed with mixed upper/lower case and must always take the
>form of a descriptive noun. Variables which are members of a class must have
>m_ prepended to the name.
A situation in which Hungarian notation is appropriate is in assembly
language code. In assembly language, there is no type checking done
by the development tools, so the programmer is ultimately responsible
for respecting the data types of all variables. In this situation it
is fair to require that type information be present in all variable
names. Portability is not an issue in this case since assembly language
is inherently non-portable. Assembly language routines are rarely if
ever used as part of a published interface so backward compatibility is
not an issue either. Changing a data type means a rewrite in assembly
language, so no flexibility is lost by requiring that a variable name
be changed at the same time as CPU instructions that manipulate that
variable.
Of course Hungarian notation _is_ used by the open-source Wine project,
for fairly obvious and unavoidable technical reasons.
>8. File level comments are to be included in all source and header files.
>Module name, author, a general description of the code or class contained in
>the file, and a copyright notice should be included as per the sample below.
>The leading and trailing lines (with all the asterisks) should be 78
>characters long to serve as a reference ruler for gauging the length of code
>lines.
Often Corel's legal requirements for things like copyright notices have to
be merged with the open-source project's requirements for similar notices.
Some open-source projects require explicit assignment of ownership of
the code to the project maintainer(s) (e.g. the GNU project). Others are
comfortable with, or even require, that copyright ownership of individual
source files be retained by their major contributors and declared within
each file (e.g. the Linux kernel).
Corel Linux and Corel non-Linux documents specify different rules here.
Corel non-Linux wants $Header$ in every file. I'm in favor of requiring
an RCS $Id$ string in the top ten lines of every file, preferably
in the top two lines (or in the case of HTML documents, in the last
two lines). This is handy if you have a small group of developers who
use CVS for everything from source code to the department's web site.
On the other hand I've seen the opposite recommendation as well from
people who have had to cope with one too many patches that modify the
expanded RCS strings, and for the Wine project we turn off RCS keyword
expansion on imported sources anyway...
>10. Function/method size: bigger is not better. Functions and methods should
>be kept to a maximum of 40-50 lines. Larger methods should be broken down
>into several smaller methods.
It would appear that this rule combined with the 78-column line length is
designed to make all functions fit on a single page of printed paper. This
is a completely arbitrary decision (why 78? Why not 130? Why 50 lines,
and not 64, or 86?) and one that can have a negative impact on technical
quality of the code. The impact of this rule on code quality can be
similar to the impact of paying programmers per line of code written.
Keep functions a reasonable size. If you actually have to count lines
or characters to determine whether a function is reasonable in size,
then you are probably not qualified to write them.
>16. Goto statements should not be used.
Neither should the passive voice be used (sorry, a little grammatical
humour there ;-).
>17. Return: functions and methods should be structured to have a single
>return.
I'd love to see how 16 and 17 are satisfied at the same time in e.g.
functions that implement a conditional dispatcher or state machine.
>19. Comparisons for equality against a constant value should list the
>constant value first. Its easy to miss one of the equals signs - this way
>the compiler will catch it if you do.
>
>eg. if (-1 == m_nResult) rather than if (m_nResult == -1)
This isn't the only instance of Corel forcing the compiler to reject
invalid code. You should see how they verify the sizes of data types!
>20. Header files must not include other header files (to prevent header file
>bloat). Use forward class references to inform compiler of required data
>types.
This would make autoconf impossible. I suspect it also causes severe
backward compatibility and portability problems as well, since you can't
abstract types properly like this, and it restricts your ability to
change an interface because all users of the existing interface must know
all the dependencies between declarations in different header files. To
work around that requires duplication of code, which is even worse.
>21. Filenames: source files should have .cpp extension. Header files should
>have a .h extension. All filenames should be created using only lower case
>characters. All filenames in #include statements should also be written in
>lower case. Source and header files should normally reside in the same
>directory. Large projects should be split up into smaller sub-projects to
>keep the number of source files in each directory manageable.
For the Wine project we run all Corel source code through a filter that
forces all filenames to lowercase anyway. This guideline isn't part of
the Corel non-Linux specifications, and there are other differences
between the header guidelines for Corel Linux and Corel non-Linux.
> * Inline methods which contain more than 1 statement must not be coded
> within the class declaration. They should instead be coded within the
> header file, immediately following the class declaration.
Does anyone understand the rationale for this? egcs behaves identically
in both cases as far as I can tell.
Of course, missing from any Corel coding style document is a good set of
rules for making life easier for people who have to do ports, especially
those who cannot use Microsoft compilers for the target architecture.
Hmmm... maybe that's why my PL wants me to do this...
--
Zygo Blaxell, Linux Engineer, Corel Corporation. zygob@corel.ca (work) or
zblaxell@furryterror.org (play). Corel may disagree with my opinion (above).
Linux ryo-ohki 2.0.36 #1 Dec 12 23:38 EST 1998 i586 up 8:15
Linux washu 2.2.4 #3 Mar 28 23:20 EST 1999 i686 up 1 day, 8:54
Reply to: