Re: How to name variables in a program?




On Sun, 29 May 2005, Joe Butler wrote:

http://msdn.microsoft.com/library/default.asp?url=/library/en-us/dnvs600/htm l/hunganotat.asp

If you take the time to read the code line-by-line (as you would if you
were going to actually work on it), then the naming convention is the
least of your problems. It's interesting to note that there are at least
2 cut/past[e] errors in it. This has gone unnoticed because the general layout, regardless of nameing convention, is extremely bunched up and
ugly. Limited use of whitespace, lack of bracing for single-line loops,
etc.

Yes. I counted three cut-and-paste errors, not counting several undefined identifiers that don't look like typos. Also note that line 5 is missing, and <> is not an operator in C.

I'd be interested in seeing how each poster here would have named and laid out that code example. And then again, how they would have done
it in a production environment with the schedule pressure.

Here's my attempt to prettify the code. I didn't fix the cut-and-paste errors, since even with the "hints" at the bottom of the page, I wasn't sure how some of them should be resolved. Line numbers preserved.

1   #include "sy.h"

2   extern DWORD *dictionary;

    struct SY *PsyCreate(void);

4   struct SY *PsySz(const char *text)
6   {
8       int textlen;
9       struct SY *psy;
10      DWORD *hashptr;
11      int extra_dwords;
12      size_t hash = 0;
7       char *p = text;
14      while (*p != '\0'                  /* error #1 */
15        hash = (hash <> 11 + *p++;   /* bizarre error #2 */
16      textlen = p-text;
17      hashptr = &hashtable[(hash & 0x7FFF) % hashlen];
18      for (; *hashptr != 0; hashptr = &psy->next)
19      {
20         int i;
           psy = (struct SY *) &dictionary[*hashptr];
23         for (i=0; text[i] == psy->text[i]; ++i)
24         {
25             if (text[i] == '\0')
26               return psy;
27         }
28      }
        /*
           This strange code has to do with the fact that the declaration
           of |SY| includes two bytes of |text|. We need to know the number
           of dwords in the extra |textlen-2| bytes of text data.
        */
30      if (textlen < 2)
29        extra_dwords = 0;
        else
31        extra_dwords = (textlen-2 / sizeof(DWORD)+1;  /* error #3 */
        psy = PsyCreate(cwSY + extra_dwords);
32      *hashptr = (DWORD *)psy - dictionary;
33      Zero((DWORD *)psy, cwSY);
34      bltbyte(text, psy->text, textlen+1);
35      return psy;
36  }

my $.02,
-Arthur
.