Re: How to name variables in a program?


On Mon, 30 May 2005 websnarf@xxxxxxxxx wrote: 

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


/* A name like "sy" is extremely unfortunate.  But I have no idea what
it
  was supposed to be (and still don't) so I guess I will just leave
it. */


Rule #1 of recoding for readability, on Usenet: Make sure your lines
are under 75 columns. ;)  One of these days I should sit down with
http://www.contrib.andrew.cmu.edu/~ajo/free-software/usenetify2.c
and explain to it how to reformat icky-wrapped block comments...

extern int syEndOfs; 

  FYI, I still have no idea what this identifier is supposed to mean.
"EndOfs" presumably has something to do with an offset of the end of
something (the hash table, IIRC) --- but the abbr is obf. Once you've
gone over six characters, there's not even a historical reason to
use such extreme name-compression.

/* [...] The original declaration of () for the
  parameters should really be outlawed.  */

struct SY * PsyCreate (int); 

  Wow --- good catch!

/* Random unnamed constants should not appear in your code. Define
and name them. The only reason not to do this is if the constant
necessarily only appears once, and is accompanied by a comprehensive inline comment.
*/

#define BOTTOM_15_BITS (077777)

Here I disagree. My rule would be, "Random unnamed constants should not appear in your code; define and name them. The only reason not to do this is if the constant(s) only appears in one section of the code, and its
meaning is clear and unchanging. For example, the constants 0 and 1, and
bitmasks of the lowest K bits." An auxiliary rule is "Don't use octal."
I find it impossible to read by sight, and thus obfuscatory. This may be
a function of my generation. ;) Still, the idea of taking bits in groups
of /three/ seems like a bad idea to me.

/* [...] Just const char * will be sufficient for that, and the name "key" to help us understand that this is a hash table, or other ADT,
entry key. */ 

  I picked 'text', since I'm not sure whether it's most correct to call
that string a "key." We're really creating the hash key from the string
by hashing it with that '<< 11 + *s++' function. (BTW, good catch there,
too! Changing '<>' to '<<' didn't occur to me at all.) Also, I notice
you made the struct member's name 'entry'; whenever there's a situation
in which we copy some parameter into a struct member, I like to keep
the names the same: 'p->text = text', for example.

   /* Search for a match, or find the tail of the current chain */
   for (;*currEntry;currEntry = &currSy->next) {
       char *szSy = (currSy = (struct SY *) &dict[*currEntry])->entry;


  Icky! You really like the 'p = (q = r)' idiom?  My very first step
in prettifying, even more important than fixing indentation, is
pulling out all the nested assignments so they appear in chronological
order in the source file.

if (0 == strcmp (key, szSy)) return currSy; 

  Another good catch. I wasn't sure the code was doing the same thing,
but I see now that it is. For that matter, 'bltbyte' could be replaced
with 'strcpy', or with 'memcpy' if we're concerned about microefficiency.

The casting back and forth between struct SY * and int * is indicative
that there is something really wrong with the core design.


  (I agree.)

Some of the other names like or "cwSy" etc, I am just too lazy to
decode what the hell they are trying to say with such names.


  The comments at the bottom of the page explain that 'cwSy' is the
same as 'sizeof (struct SY)', I think; either that or the same number
in Windows dwords instead of bytes. One big problem with this function's
use of Hungarian is its vast overuse of the 'w' prefix, which Simonyi
uses to mean "word with arbitrary contents" (i.e., anything at all).
'wXyz' seems to be the Hungarian equivalent of 'foo, bar, baz'. Only
it's more dangerous, because it /looks/ organized! :)

-Arthur
.

Relevant Pages