CodeStyle

$Id: CodeStyle,v 1.9 2001/02/01 17:56:04 broeker Exp $

The following things have be observed when writing new code for gnuplot:
(this file is currently under construction) Some of the following is
just personal bug-bears, so dont take any offense if something I
moan about is something you think is good style...

These rules were originally written up by David Denholm, and I keep
updating them as I go along. -Lars


MISSING FUNCTIONS, AND FUNCTIONS TO BE AVOIDED
----------------------------------------------
The following functions may not be used, since they are not present on all
systems (though the substitute functions might be defined to exactly these
functions)

function	use instead

bcopy		memcpy
bzero		memset
index		strchr
rindex		strrchr
strncasecmp	strnicmp



 The number of macros for conditional compilation is getting a little
extreme! I (personally) think it's better to make the conditionally-compiled
code 'feature-based' rather than 'compiler-based'. I think this is particularly
true for the many DOS compilers. The sort of thing I am thinking of is,
for example, whether to disable hidden3d stuff, or whether to store
data points as float rather than double to save space. Rather than having
a long list of compilers or OS's which use one or the other, add macros
such as SMALLMEMORY or NOHIDDENSTUFF and define these in the makefiles.
 Perhaps a sensible guideline for choice of such macros is to arrange
things so that the smallest number of ports are affected. For example,
if we only compiled the hidden stuff if HIDDENSTUFF was defined, most
makefiles would have to be updated. But if instead it is being disabled
for a few machines, only those makefiles have to explicitly define
NOHIDDENSTUFF.

 Perhaps a good guideline would be that gnuplot should build cleanly
with most available features if no macros are defined on an ANSI C compiler
on a 'typical' (POSIX, say) unix machine.

 For example, I myself have broken this rule by requiring the macro
HAVE_LOCALE in order to support setlocale() - this feature is available
with ANSI compilers, and so setlocale() calls should be enabled by default,
and disabled only if NO_LOCALE is defined at compile time. Does this
sound reasonable? For example, there was some code in fit.c that would
prefer to use tempnam() if it is available, but falls back to ANSI fn
tmpnam() if necessary. The way it was done, tempnam() was the default,
and there was a list of those systems that had to use tmpnam().
But the trouble was that a new one had to be added to the list every
few weeks as new machines were discovered. The current scheme is
that tmpnam() is used unless feature HAVE_TEMPNAM is enabled.



On a related note... if one particular machine does doesn't provide
a standard-ish function, but the same functionality can be
acheived, it would be preferable to implement the standardish
function in a os-specific source file, so that the core code can
simply invoke the function without any conditionals, and the OS-specific
files provide the missing functions. For example, in fit.c (at the
time of writing) where a temporary file is needed, there is some
inline code for DOS, OS2, AMIGA, etc to create temporary files,
otherwise tempnam() is used. I think I'd rather have tempnam()
implemented as a function in dos.c for example, then the fit code
would not need to have any conditional code other than HAVE_TEMPNAM



Also, think generic where possible... I once noticed that popen()
had been implemented for atari or similar using temporary files
and system() call. It seems to me that this could easily be done
as a generic solution, so that DOS ports, for example, could also
benefit.




FUNCTION PROTOTYPES
-------------------

Function prototypes are mandatory, even for local functions that are
declared before use. This is necessary for a clean compile on
some machines. gcc -Wstrict-prototypes is recommended.
However, to make the code compilable on pre-ANSI style compilers,
prototypes have to be enclosed in a special macro, e.g.

int strcmp __PROTO((char *s, char *t)); /* note the double ()'s */

Probably from gnuplot release 3.7.1 on, I will require that
all function declarations and definitions are in ANSI style.
I see absolutely no point at all in ignoring the benefits of
ANSI compilers, almost ten years after this language became
an ISO standard.

int
strcmp(char *s, char *t)
{
 ...
}

On platforms which use the included configure script, the ansi2knr
tool in the src subdirectory is invoked transparently if the compiler
doesn't support prototypes (Ultrix, SunOS 4.x). Other platforms may
require explicit rules or additional makefiles for non-ANSI/ANSI
compilation. The man page for ansi2knr is included. Note that for
ansi2knr to work, the function return type must be on a separate line,
or, to quote from the ansi2knr manual, "ansi2knr recognizes functions
by seeing a non-keyword identifier at the left margin, followed by a
left parenthesis, with a right parenthesis as the last character on
the line."



While compilers do not require that explicit declarations be
given for integer arguments, we do !


While ANSI compilers can use prototypes for implicit typecasts, k&r
compilers do not have this information. Avoid relying on implicit
conversions of function parameters. gcc -Wconversion helps with this.
There are many signed/unsigned warnings, but look out for other
ones which may be more serious, in particular integer to float and
vice versa. Placing casts is necessary in this case for correct
code on non-ansi compilers.

[we will definitely give up k&r support in the near future, but
 since existing code seems to work with k&r, we're sticking
 with it for the time being.
]




INTEGER SIZE
------------

Large integer constant expression have to be explicitly cast to long, even
if the result is assigned to a long variable.

long t=60*60*24;
results in a overflow on 16 bit compilers, even though the result fits into
the long variable.

Correct: long t=60l*60l*24l;



Similarly, though not particularly important, ANSI and k&r compilers
treat integer constants > MAX_INT differently. If you mean an
unsigned integer constant, say so.




Please avoid duplicating large sections of code - make the effort
to make a function or macro out of the common code.


min(a,b), max(a,b), MIN(a,b), MAX(a,b) are all predefined by some
compilers. I am now using GPMIN() and GPMAX() [wot a pain !]


Avoid putting directories into #includes - eg #include "term/file.h"
is to be avoided. Instead, #include "file.h" and use -Iterm on the
compile line.


coordval is typedef-ed to either double or float - it is almost always
double, but please take care not to mix coordval's with doubles.


An important rule unknown to many, it seems: *never* pass a 'char' to
any of the <ctype.h> functions, as-is. Don't cast it to (int), either
--- that would happen automatically, anyway. You *must* cast to
(unsigned char), instead. Otherwise, there'll quite likely be crashes
with 8-bit characters on systems where 'char' is signed.


LAYOUT AND INDENTATION
----------------------

 The code layout is getting into a bit of a mess, due to mixed
conventions. IMHO the only useful style is one tab stop per
indent. This way, anyone can set their editor to display
with their preferred spacing. More importantly, one has to
delete only one tab to outdent once more : this makes it so much
easier to ensure alignment even if the opening brace is
off the top of the screen.
 Much of the code seems to assume tab=8, and uses 4 spaces,
then one tab, then tab+4, then 2tab, ... On an entirely
personal note, this breaks my folding editor :-(

 I think vi does this by default. If using vi, please try
putting set ts=4 into your ~/.exrc file.


 Unfortunately, gnu indent does not seem to recognise this
as a layout style. If it did, I'd have run all sources
through it long ago. [GNU indent -kr -cp0 -l132 -lps -br -psl
is what I use. Very little manual editing is necessary after
running this, mostly for long (> 80 cols) lines. Anyway,
I don't care which indentation style _you_ use, I'm running
all code through indent and be done with it. -Lars]



Please use lots of vertical whitespace between unrelated
blocks of code in functions. And it should not be need to
be said, but I'll say it anyway... please put in lots of
comments describing blocks of code. Many people maintain and
contribute to this code.


 The functions in plot?d.c and graph*.c are sometimes very long.
This isn't really a problem, since there's a lot of semi-independent
things to be done in sequence. However, I do object to the
practise of having all variables declared at the top of
such functions. Please keep the scope of temporary variables
as local as possible, particularly in these long functions.
The main reason is for ease of maintenance : many people
make modifications to small parts of code without fully
understanding in exacting detail the surrounding code.


In case you were wondering, lines of the form /*{{{ comment */ 
and /*}}}*/ are markers left by my folding editor when I am forced
to fold up particularly long functions when trying to understand the
logic. I tend to leave these markers in when I finally figure it out,
though perhaps I should not.


Source code is intended to be read and maintained by humans. As such,
readability is prefered over elegance. Please separate all operators
from operands with a space, ie. instead of x+=3, use x += 3. The former
is not only less readable, it may also break older compilers! This rule
should also be followed for simple assignments, ie. in for (;;) loops.
Unary operators should be writen as usual, ie i++.


No C++ style comments (//).
No trailing comments on #ifdef/#ifndef lines.
No #error and #warning directives.

[more to come]