Slash'EM coding conventions

Introduction

Slash'EM has evolved over many years and has been developed by a huge range of people. One of the consequences of this process is that different parts of the code follow slightly different coding conventions. This makes coding in the Slash'EM environment particularly challenging, but the following pointers may prove helpful:

• When modifying existing code, follow the coding conventions that it has been written in.
• Don't try and change existing code to follow your conventions (or indeed those laid out here). You may feel that your conventions are superior (and you may even be right), but patches are likely to be rejected if they do this. Alternatively, a member of the dev-team may have to spend time redoing your patch following the conventions of the surrounding code.
• Slash'EM is based on NetHack. When a new version of NetHack is released, we have to spend quite a lot of time integrating the changes into Slash'EM. This process is made considerably easier if the Slash'EM code is identical to the NetHack code (because patch can then automate it). Consequently, we try to follow the NetHack codebase unless there are good reasons to deviate (ie., because we want a different behaviour).

Module layout

When writing a new module, you should use the following ordering:

1. An RCS ident
2. A copyright section. You may choose to donate the copyright to the Slash'EM dev-team, or if you prefer you may keep it for yourself. In either case, the copyright statement must include the years which apply and must end with the following license statement: "NetHack may be freely redistributed. See license for details."
3. Any include statements. You should use angle brackets for system include files and quotation marks for include files provided by Slash'EM.
4. A prologue detailing the purpose of the file and an overview of what it does.
5. Any pre-processor defines, structure definitions and typedefs that should remain local to the module.
6. Definitions of any global data.
7. Functions. Try to order these so that functions of a similar abstraction level are grouped together and so that higher level functions come earlier in the module.

Data structures

• Structure definitions should begin on the first column and have the opening brace on the first line. Fields should be indented with one TAB, be commented and normally consist of one field per line (tightly related fields should be listed on the same line). Example:

  struct robot {
  	int charge;	/* Amount of battery life remaining in turns */
  	int type;	/* Robot type */
  #define ROBOT_TURTLE	0	/* A wheeled robot with very simple movement */
  #define ROBOT_ANDROID	1	/* A humanoid robot capable of walking */
  #define ROBOT_ARM	2	/* A robot which is fixed in place */
  	int x, y;	/* Current position */
  };
  

• Variables which rely on being initialised should be done so explicitly (rather than relying on C's default initialisation to zero). There should be a space before and after the equal sign. Aggregate types (structures and arrays) should be fully parenthesised where nested. Where space allows, initial values should be listed on one line. Where it does not, each field (or each element) should be listed on a separate line indented by half a TAB (four spaces). Examples:

  int done = 0;
  const char *prompt = "What?";
  struct robot robots[] = {
      { 0, ROBOT_TURTLE, 0, 0 },
      { 10, ROBOT_ANDROID, 0, 0 }
  };
  

• Global variables should normally be declared static unless there is a clear need that cannot be accomplished otherwise. Where modules do need to share variables, this should be commented clearly.

Comments

• Longer comments should be written as block comments with an opening "/*". The text of the comment should be placed on the following lines, with a "*" at the start of each line aligned with the "*" on the line. The comment should be closed with a line containing "*/". Comments inside functions should be indented to match the code they are describing. Example:

  /*
   * This module implements a truly amazing feature.
   *
   * You just need to call wave_magic_wand() and all your bugs are fixed.
   */
  

• When commenting code, avoid explaining what the code does. If the code is so complex that it needs such an explanation, then you should probably re-write it. Instead try and explain why the code is written that way.
• Shorter comments should precede the code they are describing and form a single line as follows:

  	if (!done) {
  	    /* Avoid alloc(0) which may return NULL */
  	    f->data = (void *)alloc(1);
  	}
  

• Very short comments can be appended to the end of the relevant line. They should be arranged so that they are aligned with one another and so that, as far as is possible, they form a separate visual group to the code:

      if (intell < 10) tmp = -1;      /* Punish low intell. before level else low */
      else if (u.ulevel < 5) tmp = 0; /* intell. gets punished only when high level*/
      else if (intell < 14) tmp = 1;
      else if (intell <= 18) tmp = 2;
      else tmp = 3;                   /* Hero may have helm of brilliance on */
  

• Don't add comments which describe the history of the file. That's what version control is for:

      /* KMH, balance patch -- new macro */
      if (obj->opoisoned && is_poisonable(otmp))
  	otmp->opoisoned = TRUE;
  

• You should tag comments which give your opinions with your initials but avoid tagging comments which describe factual matters. This will make it easier for future maintainers to update such comments.

  /* Not totally the same as for real monsters.  Specifically, these
   * don't take multiple moves.  (It's just too hard, for too little
   * result, to program monsters which attack from inside you, which
   * would be necessary if done accurately.)  Instead, we arbitrarily
   * kill the monster immediately for AD_DGST and we regurgitate them
   * after exactly 1 round of attack otherwise.  -KAA
   */
  

Functions

Non-trivial functions should start with a block comment that gives the name, a short description of what the function does, a discussion of any non-trivial design decisions and a discussion of the return value (if needed). It is normally better not to describe the implementation (which should be clear from the code anyway) since this normally just creates work for future maintainers.

• Local functions that are called earlier in the file than they are declared (which is likely in the top-down function ordering preferred by NetHack) should be declared using the STATIC_DCL pre-processor definition and one of the DECL macros.
• Every function should be defined with an explicit return type (don't rely on C's default of int and define functions that don't return a value using void). Functions which are local to the module should be defined as static using the STATIC_OVL pre-processor definition.
• The return type should be on a line by itself with no indentation.
• The next line should contain the name of the function and its parameters in "old-style" (ie., without prototypes). Where modules in the sys or win directories are for a platform where all compilers are known to be ANSI-C compatible, prototypes may be used instead.
• Each parameter should be declared (don't rely on C's default to int) and should normally be one per line, commented and without indentation (parameters which are closely tied may be listed on the same line).
• The opening brace of the function definition should be on a line by itself and in column 1.
• The contents of the function definition should be indented by one step.
• Finally, the closing brace of the function definition should be on a line by itself and in column 1.
• External functions should be declared in extern.h using one of the DECL macros.
• Functions should return zero on failure and non-zero on success.

Local variables

• Local variables should normally be defined at the top of each function (occasionally it can be useful to use variables which are local to a compound statement, but most of the time it reduces readability).
• Local variables should not hide the definition of global variables.
• If a function uses variables which are shared with another module then they should be declared with the extern keyword at the top of the function. Variables which are more widely used should be declared in decl.h instead.
• Each variable definition or declaration should be on a separate line with a comment describing how it is used in the function.
• The declaration list (if present) should be separated from the statement list of any function or other compound statement by a blank line.

Statements

• Statements inside a compound statement should be indented one more level than the statement which they fall within. Each level of indent should be half a tab. Lines which are indented by an odd number of levels should start with the required number of tabs followed by four spaces for the remaining half-tab.
• The opening brace of a compound statement should be at the end of the line of which begins it.
• Switch statements must not allow control to run off the end of the statement list. Instead, they should normally end with a break statement.
• Switch statements that have code falling through from one group of cases to another have a comment at the fall through point to indicate that this is deliberate.
• Flow-of-control statements which take an expression in parenthesis should have a space between the keyword and the open parenthesis to provide a visual distinction between functions. Note that sizeof should not have a space since it is classed as a function for this purpose. Return statements should not have parentheses around the value that is being returned.

	if (expr) {
	    statement;
	    statement;
	} else
	    statement;
	switch (expr) {
	case 1:
	    statement;
	    /* Fall through */
	case 2:
	case 3:
	    statement;
	    break;
	}

Expressions

• The obsolete assignment operators =+, =- etc. should not be used.
• The comma operator should have no preceding space and one following space and should only be used for parameters or where the statement cannot be split into two statements.
• The . and -> operators should have no surrounding space.
• All other binary operators should normally have both a preceding and a following space. This can be relaxed for trivial expressions deep inside complex expressions where using no space for the inner expression can aid readability.
• The conditional operator ?: should have a space before the "?", between the "?" and the ":" and after the ":".
• Unary operators should have no space surrounding them.
• Function and macro calls should have no space between the name and the opening parenthesis.
• Assignment expressions whose value is used other than as the right-hand side of another assignment expression should be enclosed in parenthesis to reassure a number of compilers that an equality expression was not intended.
• Apart from this, expressions should not include redundant parenthesis. They may improve readability for new programmers but they make it harder for experienced programmers to read the code. If an expression seems too complex to you then break it down using macros or temporary variables if needed.
• Lines should be no longer than 80 characters. Where statements need to be broken over two or more lines try and choose suitable points so that sub-expressions are not split over two lines unnecessarily. Where expressions can be broken either before or after a binary operator, then the most important thing is consistency within the statement and with nearby code but the default should be to break after binary operators.
• Continuation lines should be indented by two levels.

    if ((k = arm_displacement()))
	return factories[next_factory].robots[this_robot]->type == ROBOT_ARM ? 
		180 * sin(alpha * ++k * eccentricities[i+j]) / PI : 0;

Macros

• Macros should be written to be as safe as possible. This is especially important where they are named in lower case.
• The definition should be written so that the substitution parameters are properly parenthesised. Remember that a substitution parameter can be any assignment expression. Your macro must ensure that it can cope with such parameters without misbehaving.
• Where at all possible guard against the possibility of side effects. This is best done by using a parameter exactly once. If this is not possible, consider using a global variable to avoid the problem. This decision, however, should not be taken lightly. If at all possible, name macros which do not guard against side effects in upper-case.
• Guard against the possibility of affecting the surrounding code. Where your macro can be written as an expression this can be achieved by simply placing it in parenthesis. Macros that cannot be written as expressions should normally be written using the do { ... } while(0) construct. Note that there is no semi-colon at the end of this construct; it will be provided by the semi-colon at the end of the macro call. This is the safest construct since there is no legal token that can follow the macro call except ";".

    #define ADVANCE_PTR(io, ptr, inc)					\
	    do {							\
		(ptr) += (inc);						\
		if ((ptr) >= (io)->buffer + sizeof((io)->buffer))	\
		    (ptr) -= sizeof((io)->buffer);			\
	    } while (0)

Naming conventions

• Do not define variables or functions with either an underscore prefix or suffix. Such names are reserved for system use under UNIX and perhaps other platforms.
• Macro names and pre-processor defines should be in upper-case (macro which may be used exactly as if they were functions may be in lower-case).
• Variable and function names and structure/union tags should be in lower case.
• Avoid having variables or functions with names that differ only in their case.
• Avoid the Hungarian naming convention.

Constants

• Use pre-processor defines for numerical constants where this would aid readability. This is especially important for magic values which are used in more than one place in the code.
• Long integer constants should have an explicit L suffix.
• Don't split long strings using ANSI style string concatenation.
• The use of NULL should be avoided. Instead use 0 cast to the relevant type.