From the NannyMUD documentation

LAST CHANGE

2000-12-23

TOPIC

NAME

        things not to do - Some examples.

DESCRIPTION

        THIS text was written a long time ago. Some of the things in
	it are still valid. Some are not. It is kept as a historical
	document.
	
	This is a list of things you shouldn't do in LPC, compiled by
	Profezzorn. Most things in here still _work_ it's just that you
	shouldn't do them anyway, because you give away the fact that you
	can't code.

	1) useless functions.
	   This is a good example of a useless function
	
	   void init() { ::init(); }
	
	   Since init will be called in the inherited object anyway if it
	   is removed.
	
	   Here is another example:
	
	   int query_value() { return 0; }
	
	   Since the return value for a function that doesn't exist is always
	   zero, this function can be removed. Even if this function returned
	   a value it it could be removed if this object inherited a standard
	   object, since it most likely has a value variable or a set_value
	   function.  Same thing goes for realm and query_inorout.

	2) useless global variables, an example:
	
	   object deer;
	   void reset()
	   {
	     if(!present("raindeer"))
	     {
	       deer="/std/lib"->make("raindeer");
	       move_object(deer,this_object());
	     }
	   }
	
	   In this example the variable 'deer' will use up memory for no good
	   reason as it doesn't store anything that will be used later. This
	   variable should be made local, or removed totally. This is how to
	   make it local:

	   void reset()
	   {
	     object deer;
	
	     if(!present("raindeer"))
	     {
	       deer="/std/lib"->make("raindeer");
	       move_object(deer,this_object());
	     }
	   }
	
	3) Clone things in reset() before setting up the description of the
	   room.  This is a bad thing, because if the cloning fails, (and it
	   will eventually) no exits will be added to the room and the mortal
	   will be stuck there.  The reset function in a room should look
	   something like this:

	   void reset(int arg)
	   {
	     if(!arg)
	     {
	       /* set long, short, light and add exits and items here */
	     }
	
	     /* clone monstes and other stuff here, but only if they
	      * weren't present already
	      */
	   }
	
	4) Don't have call_outs running when there isn't players around.
	   All call_outs that do call_outs to themselves should either have
	   REALLY long delays or stop when there hasn't been a player around
	   for a while.  This is to preserve cpu. For walking monsters, this
	   is done automatically if you use the walk functions in
	   /obj/walkingmonster.

	5) Constant arrays/mappings.
	   It is tempting to write things like this:
	
	   #define FOO_MAP ([ \
	     "home", "/players/profezzorn/workroom", \
	     "away", "/players/profezzorn/bungalow", \
	     ])
	
	   and then use it like this: FOO_MAP[cmd] in one or more places in
	   the code.  Avoid this! LPMUD implements this very inefficiently and
	   it might take seconds to get the value if the array is large. There
	   are two simple solutions, either you use a switch() instead, or you
	   store the mapping in a variable somewhere and use that. (only once
	   please).

	   Note that doing:
	   mapping foo=([
	     "home", "/players/profezzorn/workroom",
	     "away", "/players/profezzorn/bungalow", \
	     ]);
	
	   (foo is a global variable) is faster, but just as bad if this is
	   anything but a room. This is because there will be one copy of the
	   mapping for each clone. (and one for the master object) For large
	   arrays/mappings this can take quite a lot of memory. Using a
	   switch() or calling a function in a room or daemon object that
	   returns the value is safe.

	6) sscanf or lower_case on command argument.
	   Don't do this:
	
	   void init() { add_action("buy","buy"); }
	   int buy(string s)
	   {
	     sscanf(s,"%d",s);
	     /* rest of buy code */
	   }
	
	   If the command "buy" is given without an argument, s will be zero
	   and the sscanf will fail with an error. Make sure you check that
	   the arguments to your command functions are nonzero before using
	   them, this can be done like this;

	   int buy(string s)
	   {
	     if(!s)
	     {
	       notify_fail("Buy what?\n");
	       return 0;
	     }
	     sscanf(s,"%d",s);
	     /* rest of buy code */
	   }
	
	   This warning also applies to functions like present(),
	   find_living(), find_player() etc. etc.

	7) Don't make things too generic.
	   doing this in a guild object:
	
	   string guild;
	   string query_guild() { return guild; }
	   int reset(int arg)
	   {
	     guild="stupid guild";
	   }
	
	   would be too generic, it wastes codespace and variable space, it
	   would be simpler, faster and use less memory to write:
	
	   query_guild() { return "stupid guild"; }
	
	8) Don't use this_object()->
	   Function calls to 'this_object()' does NOT need to use the arrow
	   syntax.  It is much neather, smaller and faster to call the
	   function directly, simply replace the text this_oject()-> with
	   nothing and all and it will still work. Example:

	   write(this_object()->short());
	
	   _should_ be:
	
	   write(short());
	
	9) Don't use find_living()
	   The function find_living() might be good for tell and locate. BUT
	   it is not good for attack spells or feelings, since it searches the
	   whole mud.  Just imagine a 'hit' command using find_living... You
	   do "hit rabbit", and the command fails because it finds a rabbit on
	   the other side of the mud. Or even worse, it doesn't fail and you
	   hit the rabbit on the other side of the mud... Use present instead,
	   then check that it is a living by calling query_living() in the
	   resulting object. (If any, don't forget to check!) Here is a small
	   example of how to do it:

	   int hit(string who)
	   {
	     object target;
	
	     notify_fail("Hit who?\n");
	     if(!who) return 0;
	
	     target=present(lower_case(who),environment(this_player()));
	     if(!target) return 0;
	
	     if(!target->query_living())
	     {
	       write("You don't want to hit that, you could hurt yourself.\n");
	       return 1;
	     }
	
	     "/std/msg"->msg("\bPRON hit\b$ \2OBJ.\n",this_player(),target);
	     return 1;
	   }