Re: Problems with write_to_output and buf (sprintf)

From: Daniel A. Koepke (dkoepke@circlemud.org)
Date: 07/30/02


On Tue, 30 Jul 2002, Mathew Earle Reuther wrote:

>     if (class_ok_by_race[(int)GET_RACE(d->character)][0]) {
>       sprintf(buf + strlen(buf), "(%s)%s  ", classes[0].class_letter,
> classes[0].class_name);

The bug: you're appending the text to the existing buffer in these
sprintf() lines.  Note that 'buf + strlen(buf)' offsets the position in
the buffer by its current length, meaning that what you're writing to the
buffer is being put at the end.  Obviously, you want to overwrite text
you've already written out, not append to it (otherwise, it remains in the
buffer and is repeated with each of your sprintf() calls).

The real proble: the entire function needs to be rewritten.  There are
numerous problems that flag this as a maintainence nightmare.  By failing
to use looping constructs, meaningful names for constants (through either
#defines or enums), and useful tables you've made the function many dozen
times longer and more difficult than it needs to be.  By failing to
separate data from its support code you're making future development
increasingly difficult.  By assuming you can't use these constructs to
present a precise layout you've complicated the matter of producing your
desired format.

Consider, instead, #define'ing the name of your supercategories (like
classes are done in structs.h) and building tables for their names and
acceptable classes.  This turns a function from what could be a hundred
lines into some data tables and about 20 lines of display code.

  const char *supercategories[NUM_CATEGORIES] = {
    "Magical Studies",
    "Runic Languages",
    "Philosophy",
    "Natural Theology",
    .
    .
    .
  };

  bool class_ok_by_supercategory[NUM_CATEGORIES][NUM_CLASSES] = {
    /* MAG  CLE  THE  WAR  ... */
    {    1,   0,   0,   0, ... },       /* Magical Studies */
    {    0,   0,   0,   0, ... },       /* Runic Languages */
    .
    .
    .
  };

  /*
   * Produces output like:
   *
   *   Please Select a Profession:
   *
   *
   *   [Magical Studies] ->
   *   [Runic Languages] ->
   *   [Philosophy]      ->
   *   [Natural Theology]-> (g)Animist  (h)Elementalist
   *   .
   *   .
   *   .
   *
   * Reader exercises:
   *  - Add a prompt.
   *  - Count available classes for category, suppress category if zero.
   *  - Better overflow checking via snprintf().
   */
  void list_valid_classes(struct descriptor_data *d)
  {
    char name[MAX_STRING_LENGTH];
    char buf[MAX_STRING_LENGTH];
    int cat, cls;
    int idx;

    idx = sprintf(buf, "Please Select a Profression:\r\n\r\n");

    for (cat = 0; idx < sizeof(buf)-1 && cat < NUM_CATEGORIES; cat++) {
      sprintf(name, "[%s]", supercategories[cat]);
      idx += sprintf(buf + idx, "%-18s-> ", name);

      for (cls = 0; idx < sizeof(buf)-1 && cls < NUM_CLASSES; cls++)
        if (class_ok_by_race[(int)GET_RACE(d->character)][cls] &&
            class_ok_by_category[cat][cls]) {
          idx += sprintf(buf + idx, "(%s)%s  ",
                         classes[cls].class_letter,
                         classes[cls].class_name);
        }

      strcpy(buf + idx, "\r\n");
      idx += 2;
    }

    write_to_output(d, buf);
  }


> The format of the display is very important, btw, which is why I'm not
> just using an incremented (grab a class number, check it, print it if
> they can be it) method.

There's nothing preventing you from reproducing the desired format while
still using a for loop.


-dak

--
   +---------------------------------------------------------------+
   | FAQ: http://qsilver.queensu.ca/~fletchra/Circle/list-faq.html |
   | Archives: http://post.queensu.ca/listserv/wwwarch/circle.html |
   | Newbie List:  http://groups.yahoo.com/group/circle-newbies/   |
   +---------------------------------------------------------------+



This archive was generated by hypermail 2b30 : 06/25/03 PDT