coding_conventions.txt

Crawl coding conventions
========================

Introduction
============

This file documents the style conventions currently in use in the Crawl
codebase, as well as the conventions that new and/or modified code should
conform to. It is explicitly not meant to be a didactic "how to program
effectively" treatise. That is something best left to books and websites,
as well as experience.

Conventions
===========

A) Indenting
------------

Generally, use 4 spaces to indent, and indent with spaces only (no tabs).
Empty lines don't need any spacing at all.

Methods
-------

If the parameter list of a method runs longer than a line length (80 columns),
the remaining parameters are usually indented in the lines below.

    static void replace_area(int sx, int sy, int ex, int ey,
                             dungeon_feature_type replace,
                             dungeon_feature_type feature, unsigned mapmask)
    {
        [...]
    }

The same is true when a method is called:

        // place guardian {dlb}:
        mons_place(MONS_GUARDIAN_NAGA, BEH_SLEEP, MHITNOT, true,
                   sr.x1 + random2(sr.x2 - sr.x1),
                   sr.y1 + random2(sr.y2 - sr.y1));


There are cases where this is not possible because the parameters themselves
are too long for that, or because the method is already heavily indented,
but otherwise, this convention should be followed.


B) Logical operators
--------------------

Conditionals longer than a line should be indented under the starting bracket.
This probably seems obvious for simple ifs ...

    if (!player_in_branch(BRANCH_COCYTUS)
        && !player_in_branch(BRANCH_SWAMP)
        && !player_in_branch(BRANCH_SHOALS))
    {
        _prepare_water(level_number);
    }

... but it should also be followed for else if conditionals.

        else if (keyin == ESCAPE || keyin == ' '
                 || keyin == '\r' || keyin == '\n')
        {
            canned_msg(MSG_OK);
            return false;
        }

Space allowing, logical connectives of different precedence should use nested
indenting.

    case ABIL_MAPPING:          // Sense surroundings mutation
        if (abil.ability == ABIL_MAPPING
            && player_mutation_level(MUT_MAPPING) < 3
            && (you.level_type == LEVEL_PANDEMONIUM
                || !player_in_mappable_area()))
        {
            mpr("You feel momentarily disoriented.");
            return false;
        }

If a logical connective needs to be distributed over several lines,
the conjunction/disjunction operators (&&, ||) should be placed at the
beginning of the new line rather than at the end of the old one.

    else if (you.mutation[mutat] >= 3
             && mutat != MUT_STRONG && mutat != MUT_CLEVER
             && mutat != MUT_AGILE && mutat != MUT_WEAK
             && mutat != MUT_DOPEY && mutat != MUT_CLUMSY)
    {
        return false;
    }

Since conjunctions (&&) take precedence over disjunctions (||), pure
conjunctive logical connectives don't need to be bracketed, unless this is
absolutely vital for understanding. (Nested indenting helps here.)

    if (you.skills[SK_ICE_MAGIC] > you.skills[SK_FIRE_MAGIC]
        || you.skills[SK_FIRE_MAGIC] == you.skills[SK_ICE_MAGIC]
           && you.skills[SK_AIR_MAGIC] > you.skills[SK_EARTH_MAGIC])
    {
        book = BOOK_CONJURATIONS_II;
    }

In a switch conditional, the case listings don't have to be indented, though
the conditional statements should be.

                switch (mons_intel(monster_index(mon)))
                {
                case I_HIGH:
                    memory = 100 + random2(200);
                    break;
                case I_NORMAL:
                    memory = 50 + random2(100);
                    break;
                case I_ANIMAL:
                case I_INSECT:
                    memory = 25 + random2(75);
                    break;
                case I_PLANT:
                    memory = 10 + random2(50);
                    break;
                }

Comparisons using the ? shortcut may be indented ...

            mpr(forget_spell() ? "You have forgotten a spell!"
                               : "You get a splitting headache.");

... but really short ones don't have to be.

            beam.thrower = (cause) ? KILL_MISC : KILL_YOU;


C) Variable naming
------------------

When naming variables, use underscores_as_spaces instead of mixedCase. Other
conventions are pointed out by example, below.

Global variables are capitalized and underscored.
Warning: there are currently many globals which don't do this.

  int Some_Global_Variable;

Internal functions are prefixed with underscores.

  static void _remove_from_inventory(item_def* item);

Functions use underscores_as_spaces, but there are currently a lot of
mixedCase functions.

  void destroy_item(item_def* item)
  {
      // - Variables use underscores too.
      int item_weight = /* ... */;
  }

There's no convention for class/struct names (yet?)

  class TextDB
  {
   public:
      // - No rules for static member functions; they're not used often anyway.
      static void whatever();

      // - Public member functions: named like functions.
      void* get_value();

   private:
      // - Internal member functions: also named like functions.
      void _parse_text_file(const char*);

      // - Member variables get a prefix.
      DB* m_db;

      // - Static member variables get a prefix, too.
      std::vector<DB*> sm_all_dbs;
  };


But structures tend to use underscores

  struct coord_def
  {
      // - Simple structures don't need the "m_" prefixes
      int x, y;
  };


D) Braces
---------

Braces are always put on their own lines.

    do
    {
        curse_an_item(false);
    }
    while (!one_chance_in(3));


If many comparisons are necessary, this can result in a number of nested
braces. These can sometimes be omitted, as long as the underlying logic isn't
changed, of course. The following assumes that the conditions are followed by
single statements.

If both the condition itself and the conditional code are single line
statements, the braces may be omitted; do-while loops, however, should
always have braces (as in the example above).

    if (item != NULL)
        _remove_from_inventory(item);
    else
        _something_else();


Otherwise, place braces.

        if (tran == TRAN_STATUE || tran == TRAN_ICE_BEAST
            || tran == TRAN_AIR || tran == TRAN_LICH
            || tran == TRAN_SPIDER) // monster spiders don't bleed either
        {
            return false;
        }


    for (int i = 0; i < power_level * 5 + 2; ++i)
    {
        create_monster(result, std::min(power/50, 6),
                       friendly ? BEH_FRIENDLY : BEH_HOSTILE,
                       you.x_pos, you.y_pos, MHITYOU, MONS_PROGRAM_BUG);
    }

Also place braces if this is only the case because of one or more comment
lines.

        for (j = 0; j < num_to_make; j++)
        {
            // places items (eg darts), which will automatically stack
            itrap(beam, i);
        }


In the case of nested if-conditionals, try to combine the conditions, e.g.
instead of

    if (A)
    {
        if (B)
            do_something();
    }

use

    if (A && B)
        do_something();

Place braces as per the conventions above.

Else, whenever if-conditional nesting can't be avoided, always use braces. I
couldn't find an example where that isn't already necessary for logical reasons,
so these should be really rare.

In a row of if-else if-statements or in a switch-case loop, the optional braces
should be used if the bigger part of statements needs braces for logical
reasons or because of one of the conventions above. Otherwise, they may be
omitted.

    if (mons_neutral(monster))
    {
        if (coinflip()) // neutrals speak half as often
            return false;

        prefixes.push_back("neutral");
    }
    else if (mons_friendly(monster))
        prefixes.push_back("friendly");
    else
        prefixes.push_back("hostile");

When for-loops are nested and the outer loop has no further statements, the
braces may be omitted.

    for (int x = 0; x < GXM; x++)
        for (int y = 0; y < GYM; y++)
        {
            if (grd[x][y] == DNGN_LAVA)
                lava_spaces++;
            if (grd[x][y] == DNGN_DEEP_WATER || grd[x][y] == NGN_SHALLOW_WATER)
                water_spaces++;
        }

The same is true for combined for- and if-conditionals as long as all
statements fill less than four lines.

    for (i = 0; i < MAX_SHOPS; i++)
        if (env.shop[i].type == SHOP_UNASSIGNED)
            break;

If the order of if- and for-conditionals is reversed, however, place braces.

    [...]
    else if (enhanced < 0)
    {
        for (ndx = enhanced; ndx < 0; ndx++)
            power /= 2;
    }


If there are more than three nested statements with optional bracing, use braces
to roughly divide them into halves. (See example below.)

Should such nested code be followed by code other than a closing brace,
leave a free line between them.

    for (int y = 1; y < GYM; ++y)
        for (int x = 1; x < GXM; ++x)
        {
            if (grd[x][y] == feat)
                return coord_def(x, y);
        }

    return coord_def(0, 0);


E) Commenting
-------------

If you feel that a method is complicated enough to be difficult to understand,
or has restrictions or effects that might not be obvious, add explanatory
comments before it. You may sign your comments if you wish to.

  // Note that this function *completely* blocks messaging for monsters
  // distant or invisible to the player ... look elsewhere for a function
  // permitting output of "It" messages for the invisible {dlb}
  // Intentionally avoids info and str_pass now. -- bwr
  bool simple_monster_message(const monsters *monster, const char *event,
                              msg_channel_type channel, int param,
                              description_level_type descrip)
  [...]

Adding explanatory comments to somewhat complicated, already existing methods
is very much welcome, as long as said comments are correct. :)

Suboptimal code should be marked for later revision using the keywords "XXX",
"FIXME" or "TODO", so that other coders can easily find it by searching the
source. Also, some editors will highlight such keywords. Don't forget to add
a comment about what should be changed, or to remove said comment if you fix
the issue.

  // XXX Unbelievably hacky. And to think that my goal was to clean up the code.
  // Identical to find_square, except that input (tx, ty) and output
  // (mfp) are in grid coordinates rather than view coordinates.
  static char find_square_wrapper( int tx, int ty,
  [...]


  if (death_type == KILLED_BY_LEAVING
      || death_type == KILLED_BY_WINNING)
  {
      // TODO: strcat "after reaching level %d"; for LEAVING
      [...]


  // FIXME: implement this for tiles
  void update_monster_pane() {}

Comments should not be used to archive old code. If something is no longer
needed, do not comment it out, simply remove it.

F) Macro Usage
--------------------

If you need to use a macro as a boolean flag, do this:

  #define SOME_DEBUG_FLAG

  #ifdef SOME_DEBUG_FLAG
    do_something();
  #endif

Not this:

  #define SOME_DEBUG_FLAG 1

  #if SOME_DEBUG_FLAG
    do_something();
  #endif

The reason for this is that it's common for such debug flags to be left
undefined, leading to the #if statement having an undefined behavior. Most sane
compilers will give undefined macros a value of '0' during such #if
comparisons, but some compilers throw errors. So, play it safe. Use #ifdef.

Another thing to watch for with such flags is this sort of thing:

  #define SOME_FLAG
  #define SOME_OTHER_FLAG

  #ifdef SOME_FLAG
    do_something();
  #elif SOME_OTHER_FLAG
    do_something_else();
  #endif

The #elif statement should read '#elif defined(SOME_OTHER_FLAG)'. If the
macro was undefined, this would lead to the same undefined behavior noted
above. If it was defined as above, the #elif would not function as expected.

G) Spaces around parenthesis
----------------------------
Prefer no spaces inside parentheses.
Put a space before opening parenthesis if it's part of a conditional or preceded by a statement.

    if (trap_count > 0)

    catch (map_load_exception &mload)

 But not if it is the parameter list of a function or method.

    _get_inv_items_to_show(tobeshown, item_selector, excluded_slot);

    tileset.push_back(tile_def(TILE_ITEM_SLOT, TEX_FEAT));

H) Parenthesis around the value of return statement
---------------------------------------------------
None. The parentheses serve no purpose other than making the statement look
like a function call.

    return true;

    return -1;

    return coinflip();

    return form_can_wield(form) || form == TRAN_DRAGON;