diff options
Diffstat (limited to 'crawl-ref/docs/develop/coding_conventions.txt')
| -rw-r--r-- | crawl-ref/docs/develop/coding_conventions.txt | 368 |
1 files changed, 368 insertions, 0 deletions
diff --git a/crawl-ref/docs/develop/coding_conventions.txt b/crawl-ref/docs/develop/coding_conventions.txt new file mode 100644 index 0000000000..132ab62320 --- /dev/null +++ b/crawl-ref/docs/develop/coding_conventions.txt @@ -0,0 +1,368 @@ +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. +Warning: This is a new convention, so much code doesn't follow it. + + 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. + + 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 +could'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() {} |