[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: not enough real programmers?
In article <37b19d82.1187858@news.pandora.be>,
Luc Van der Veken <lucvdv@null.net> wrote:
>But with the exception of maybe one question (the C++ one), the
>answers you'll get won't give an indication of whether the people
>you're interviewing are [technically] competent for doing the
>job.
Actually, they will. Not a definite proof, but a good indication.
>What you will get is a collection of the best salespeople (those
>who are best in verbal expression and presentation), instead of
>the best programmers.
Except that if they present obviously bogus ideas, you know there's
a problem.
>As for the last question, an example: there's a huge difference
>between saying "[among other things] a good programmer writes
>code that's maintainable", and actually writing maintainable
>code.
True. You need to push a little.
>// Watch this: iSize must be > 0 !!!
>ixOut = 0;
>for(ixIn=0; ixIn < iSize; ixIn++)
> if(InArray[iInIx]) OutArray[ixOut++] = InArray[ixIn];
>return ixOut; // Return new used size
>-------
>for(i=j=0;i<n;i++)if(a1[i])a2[j++]=a1[i]; return j;
>-------
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".
People who litter their code with additional clutter are wasting a lot of
time.
Don't believe me?
PeopleNoun PNWho VerbLitter TheirAdjective NounCode PrepWith
AdditionalAdjective ClutterNoun // note subordinate clause // AreVerb
WastingGerund ArticleA LotNoun PrepOf NounTime.
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.
>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
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.
> if(InArray[iInIx]) OutArray[ixOut++] = InArray[ixIn];
Furthermore, the long names made you use the wrong name for the first
subscript. 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.
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.
>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.
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.
HTH. HAND.
-s
--
Copyright 1999, All rights reserved. Peter Seebach / seebs@plethora.net
C/Unix wizard, Pro-commerce radical, Spam fighter. Boycott Spamazon!
Will work for interesting hardware. http://www.plethora.net/~seebs/
Visit my new ISP <URL:http://www.plethora.net/> --- More Net, Less Spam!