Re: not enough real programmers?

[lucvdv@null.net (Luc Van der Veken)]

Peter Seebach posted:

> I would consider both to be "bad" code.  I think the second is more
> trainable.  'i' is a better name than 'ixOut' for the same reason English
> has words like "it" or "he".

I agree - I overdid it with the intent of making the principle
clear, in fact in my real code my loop counters are usually i,j,k
etc., and ix, iy, iz etc. when they're indexes.

OTOH, in my larger sources, you might encounter *really* long
names for global variables and functions - things like
AllowSendSingleSetting (an actual example, and not even the
longest - no leading 'b' because it's obvious enough that it's a
boolean value).

In a real specimen the variables and arrays wouldn't be called
what I called them now - but not something generic like "dest"
and "src" either, unless when the function itself was a real
generic one.

> Let's review:
> 
> >// Watch this: iSize must be > 0 !!!
> 
> This comment should have been directly on the line with the for loop,
> probably, but in any event, it's a stupid warning, and possibly wrong.  If
> iSize is less than zero, Nothing Happens, and we return 0 - indicating that
> we did nothing.  This Is Correct.  So the warning is wrong.

You're right - I don't know where I got the idea.
I've got a habit of keeping my lines short though, which is why I
put longer comments before the code they apply to, rather than
letting them run into columns 70+.

> >ixOut = 0;
> 
> 'ixOut' is undeclared, but it's a stupid name.  "ix" tells you basically
> nothing.  "Out" isn't a very good name, and the length of the name will impair
> reading later on.  I'd probably do

For a short-lived variable, it's too long, I agree.
But for the rest:
- I thought the lack of '{}' would make it clear that this was
intended as just a snippet of a larger routine, and I hate
declaring variables in the middle of code.  You'll never find
something like "for(int i=0;..." in something I wrote.
- ix is a prefix I often use to indicate that it's an Integer,
and an indeX at the same time.  Out indicates that it's an index
into the outgoing data (this could have been ixDest or so as
well, but I find Out more readable - just a matter of taste).

> 	int j = 0; /* count of used items in new array */
> and let the reader's natural ability to understand pronouns do the magic.
> 
> >for(ixIn=0; ixIn < iSize; ixIn++)
> 
> Same problem with poorly chosen names.  Gratuitously hard to read.

You have the right to have your own opinion, but in the middle
(or at the end) of a complex routine, I prefer having variable
names that tell you what they are, and what they're there for.

> >   if(InArray[iInIx]) OutArray[ixOut++] = InArray[ixIn];
> 
> Furthermore, the long names made you use the wrong name for the first
> subscript.

No, actually it's changing the name after I wrote the code and
forgetting one that did that :-)

>   Use simple names because both reader and writer will remember them
> better.  It's fairly unusual to be discarding zero-valued array elements
> (after all, if location didn't matter, why were they in the array), but I'll
> assume it's intentional.

See previous posts in this thread - I didn't invent the exercise.

> Same problem with array names. "InArray" tells you nothing you can't see when
> you read the code, but it makes it harder to read the code.  I would have done
> 	if (src[i])
> 		dest[j++] = src[i];
> which would be *much* more informative.  Also note that indentation can help
> you make your intent clear here.

Superfluously breaking up lines (what you demonstrate here IMO)
actually makes the code less readable.

It's exactly to avoid reading errors that I try never to split
single conditional statements up over two lines.  Indentation, in
my sources, means either that it's a new code block (like the
body of a loop or multiple statements in an 'if' body), or that
it's a continuation of a long statement (if, and only if, it
won't fit on one line - but as I said, I normally try keep my
lines short).

This whole thing is best described in words as a single "if it
ain't zero, copy it" - so I deliberately try to keep it on a
single line.

BTW, it was indented from the 'for' - the only thing I did
against my own habits was not indent the for line from the left
margin (where the function declaration would be), because my
newsreader would wrap the lines too early.

In real code, it would have looked something like

int func(parameters)
{
declaration;
declaration;	// avoid two on one line, unless they're trivial

   do something;
   do something else;

...

   for(the whole bunch)
      if(it ain't zero) copy it;

   return something;
}

With longer items in the statements (not uncommon in C++), it
might have become

   for(the whole bunch)
      if(it ain't zero)
         copy it;

or it might have been

   for(all) if(nonzero) copy;

in each case with a blank line before and after to indicate that
it's a separate entity, and with one or two spaces between the
subparts if they're on one line.

I'm not saying that this is the best style, not even a common
one, but it's the one I use (consistently) and I'm actually quite
happy with it.

> >return ixOut;		// Return new used size
> 
> Did I mention you're using C++ comments in what appears to be C code?  If
> this had been intended as a C++ program, you would have used
> 	for (int ixIn = 0; ...
> 
> so I assume you're just being careless with comment styles.

See above for my feelings about mid-code declarations.
IMO, only sloppy programmers (who forgot that they would need
them when they were declaring variables, and are too lazy to move
the cursor back up) use them.  A "for" statement is burdened
enough with the elements it needs of its own - adding a
declaration only makes it less readable.

I often notice that a large part of my C++ code would compile on
a C compiler (if it understands C++ style comments, like most do
nowadays), but maybe that's only because I was already
programming in C before C++ appeared on the scene.

Today I prefer to use C++ BTW, also when C would do nicely,
mainly for its strict type checking - so that's what I was doing
here too.

> Over all, the "terse" example was flawed, but less flawed.  It gets demerits
> for poor naming (a1 and a2 don't tell you which direction you meant to go in)
> and for complete lack of indentation.

The same is true for i and j - and that's how it was *supposed*
to be :-)

But if you say it was *less* flawed, I probably wouldn't like to
[have to] maintain your code (at least not anything above 1000
lines).