Fog Creek Software
Discussion Board




Evaluating Programmer Talent - Is It This Easy?

Yesterday I came up with a theory: poor programmers don't indent their code properly [or consistently].

Someone who isn't writing their code to be read isn't planning to read it again (hack it up and move on) and doesn't care about anyone else reading it again.

This might be a pretty easy to "measure" metric that's actually accurate. I can't think of an example that disproves the theory - i.e. poorly indented good code/poorly indented code from a good developer.

I guess this is an instance of The "Broken Window Theory" mentioned in the Pragmatic Programmer book.

Discuss. (Inconsistent indentation drives me up the wall.)

Walter Rumsby
Wednesday, July 02, 2003

I would say that assumption is slightly incorrect.

Someone who does not indent their code can be assumed to be a bad programmer. However, there are plenty of bad programmers who do indent their code, so you can't assume all bad programmers don't indent.

There are also plenty of good programmers who are not suited to the task at hand, which in that situation makes them a bad programmer.

So to answer your question, no it isn't that easy!

Pete J
Wednesday, July 02, 2003

I think Walter may be right.  Most of my career has been spent fixing up other peoples code and I have never seen a bad program have good indendation or commenting - ever.

I think that's part of the idea behind Python's whitespace as syntax.

Matthew Lock
Wednesday, July 02, 2003

ever heard of pretty print?  I like 1TBS because it folds nicely in my editor.  But the codebase I work with is strictly Whitesmiths  so I transparently convert when I open/save, and no-one knows.

Although I hope I'm an 'ok' programmer, I have been guilty of rushing through some function bodies in a way that could be described 'bad', and taking time to lay out other code while in the end feeling I'd got the algorithm sub-optimal.

constructive comment
Wednesday, July 02, 2003

I think the assumption between the theory is that there is only 1 type of bad programmer.  And this is flawed.

Some bad programmers are sloppy, and lack of indenting or useless commenting may be an indication of this.

Some bad programmers are so anal-retentive that they just don't get the job done, and are so fixated on the rules - that the rules become an obstacle, not help, to success. I'm sure you know the type - the guy who has to revise everything so it is just so - but ends up with nothing work.

Some bad programmers would be good programmers, if they learnt not to bite off more than they can chew in one go. Their projects go wrong, because they are unable to break up their projects into manageable lumps.

Some bad programmers are good programmers, some of the time. and vice-versa.

Some bad programmers are only bad in certain problem domains.  There are just some people who don't, and never will, get low level stuff, high level database type stuff or UI concepts.  The best low level programmer that I ever knew wrote incredible code that even mortals like me could follow (although never as well as him thanks to his feel for the subject) -- but ask he to do a dialog box or write a user understandable error message - no way, and he knew it.

Some bad programmers are bad because they never take advice from more knowledgeable individuals.

Some bad programmers are bad because they do take advice even though they know it's wrong.

Some bad programmers are bad because they revent everything, and don't research what's already been done that could help them.

(and I could go on)

Some *really* bad programmers, just don't know how to program. They fake it until they make it - but never make it!

With the exception of the last one, I think every good experienced programmer should note at least hint of familiarity - as regards their own work - in most/some of the above.  I think the best programmers know that they themselves are bad programmers, at least some of the time.  If you ain't self-critical and aware of your own limitations - then I don't think you are a good programmer at all.

So my last category of bad programmer: somebody who thinks they know everything already, so doesn't analyze and try and improve their work.

S. Tanna
Wednesday, July 02, 2003

You guys don't use editors that automatically & consistently indent your code?  I use visual studio & VI, both of which handle this task automagically for me.

         
Wednesday, July 02, 2003

Once I started using emacs I became a good programmer, by the indenting standard at least.

The Real PC
Wednesday, July 02, 2003

I agree wholeheartedly with you S. Tanna. Indeed one notoriously bad programmer that I worked with in the past would cause tidal waves through source control as they checked out every piece of code, modifying almost every line (making version diffs a nightmare), because they preferred the scope marker on the beginning of the following line rather than the end of the preceding line (the classic "where to put the the curly brace" debate, though in this case it was Delphi so it was begin and ends): Rather than actually solving problems, or adding value, their net contribution was code reformatter from one defacto standard to another defacto standard, bringing historic code up through the many standards (the great thing about standards: There's so many to pick from), because apparently their capabilities were so marginal that the placement of a scope marker was what really threw them off. I'm sure that they perceive themselves to be a world class programmer because of the excellent reformatting that they've done on everyone else's code, though they might find the job market a little rough when they go up against dozen line perl scripts.

On the same theme, another notoriously bad programmer (who was also a sociopathic liar to such a level that it boggled the mind, but that's beside the point) would refer to every half-baked, misunderstood idea as "Enterprise Programming". Again they assured themselves that they were a tremendous developer because they could constantly take MSDN articles out of context (speaking of that, see my article in the July isse of MSDN Magazine... http://msdn.microsoft.com/msdnmag/issues/03/07/ScalableVectorGraphics/default.aspx), applying n-tier methodologies to unclarified needs with dubious specifications and no defined interfaces to build around (in other works n* the rework everytime there was a realization that the "solution" was for the wrong problem, which was frequently).

In both cases there were people who grasped onto what could be good programmer practice, and could fool the casual observers, but in both cases the execution by incapable individuals just led to disaster.

Dennis Forbes
Wednesday, July 02, 2003

S. Tanna - well put.

Walter - While I largely agree with S. Tanna's response, I think that ragged indentation is a pretty common symptom of one type of bad programmer. This is the type of programmer who "pieces around" with small sections of code and tries to fix small localized problems without being willing or able to drop back and see the entire bigger picture. I've actually seen bad and inconsistent indentation in instances with several different bad programmers. These tend to be the programmers whose absolute last tendency is to gut an entire poorly constructed application and rewrite it where called for. They don't take responsibility for the entire job, only piecework.

On the other hand, I don't see many (any) instances of "good" code badly indented. Good developers take more pride in their work than to allow it to look sloppy.

So, I think you're onto something, but I don't think it's a comprehensive test, because as pointed out, well indented code could be bad in some general or structural way and could simply be pretty printed into good indentation. I've seen this, but more rarely.

Bored Bystander
Wednesday, July 02, 2003

The problem is that many organizations attempt to improve the quality of code by "standards" (such as the placement of scope markers, the number of spaces that comprise an indent, etc), but these standards don't correspond with the historic standard(s) used in previous code. Thus, if you go in to fix a piece of code or add a bit of functionality you're at a decision point: Do you alter all of the existing code to conform with the standard-of-the-day (ignoring that when that code was created, it likely followed the standard of that day), do you do your alterations following the standard evident in the historic code, or do you put in your new section following current standards (hence it'll be "ragged" with the rest of the code). In my personal opinion, option 1 is a _horrible_ option, but it's one taken too often : Instantly historical differencing is destroyed, someone wasted a large amount of time for something of dubious value, and worse yet you've mucked with working code for no particular reason, and even if you only changed the indentations of all of those functions, you had better do function/unit testing on the entire project all so that you can smugly be self-satisfied that you've imposed some control-freak imposition over existing code. Option 2, following the historic standard of the code, is personally the best option but too many programmers are too egotistical for this.

Regarding the comment that bad programmers don't know when to rewrite it all, I'd turn around and say that he WORST programmers go from project to project, constantly rewriting it all (usually slandering any prior programmers in the process). It's easier to ignore what has been done and why it was done, and is far more glory to proclaim that all that's gone before is crap and it all needs to be rewritten. The overwhelming majority of the time these rewrites are a disaster (as Joel has commented upon).

Dennis Forbes
Wednesday, July 02, 2003

As one last comment, "ragged" code can result from differences of the setting of the size of a tab in each users IDE: i.e. If Bob has tab set to 3 spaces, and Tom has a 32" 2560x2048 monitor has it set for 5 spaces, then if they don't have their IDEs set to save tabs as spaces the code will look ragged when they pull up each other's code. To either user their own code looks perfect, but because of configuration differences each will think the other's code is bad code (as people often vary between tabbing and manually spacing in), when in reality it's just a difference of settings.

Dennis Forbes
Wednesday, July 02, 2003

Dennis: excellent points.


On lack of rewriting and messy code: what I was describing were underachieving, docile programmers who are handed a piece of badly engineered code that *never* worked right in the first place. I've seen in at least two separate instances programmers just tiptoeing through the code, reluctant to rethink anything.  Of course, when the overall conception of the code is bad, the programmer will be bandaging multiple places in such code; as they go they become demoralized and they stop caring about minimal craft issues like making the indentations reflect the loop and block nesting structure.


On maintenance and multiple programmer's tab settings producing raggedness: software development starts out nice and clean, but most commercial software starts to look like a mess after a few years.  So it's unfair in a lot of cases to hang a label on production code that has seen several different phases of maintenance...

Bored Bystander
Wednesday, July 02, 2003

Another thing, definition of bad programmer, will vary depending on the organization, time, phase of the project, and outside factors.

Is a bad programmer one who develops sloppy code, hard-to-read code, delivers too late for the deadline, code that runs slow, takes too many risks in changes [making un-necessary changes to a source tree], does rewrite stuff, doesn't rewrite stuff, breaks the build, exceeds user requirements, underflows requirements, doesn't implement as spec, sticks too rigidly too the spec etc....  It depends on where in the project you are, and what's important at any particular moment

Sometimes the "best" programmers suddenly because the worst, when you are nearing the end of a project or deadline, and don't want to mess with stuff that you shouldn't.

Disclosures:

- I think that I am good programmer,  most of the time.  Exception would be hardware-interfacing.  I have done it before, and actually did it quite well (for a while I got stuck as "the guy" to do this at one company). But if I had to do it now, I'd turn down the job, or if forced, deliberately mess it up :-)

- I probably get 99% of my braces right.  I used to be anal about getting them 100% right. Now when cutting/pasting code, I don't always fix them immediately, provided after the move, the code is clear even if braces are wrong (and no, I don't fix other people's braces!)

- I am anal about white space, both vertical and in long expressions.  Again I won't add them into other's code unless I have a reason.  But it costs nothing in most languages and used reasonably well does make code easier to read.

- Years ago, I worked with a guy who has unbelievably anal about commenting style. Some of it stuck (I won't fix other people's code).

I really do think this is best commenting style in C++ (it's kind of an adaption of how we used to write turbo pascal.

/*
    Some overall explanation of intent of next section of code
*/

    // where needed comment about individual lines or fragment
    code
    code
    code

    // where needed comment about individual lines or fragment
    code

    // where needed comment about individual lines or fragment
    code
    code
    code

/*
    Some overall explanation of intent of next section of code
*/

    // where needed comment about individual lines or fragment
    code
    code

    // where needed comment about individual lines or fragment
    code
    code
    code

    // where needed comment about individual lines or fragment
    code

S. Tanna
Wednesday, July 02, 2003

> /*
>     Some overall explanation of intent of next section of
> code
> */

I used to do this, but when I needed to comment out a large section of a module, it would become a major PITA.

I now prefer to use

// Some overall explanation of
// intent of next section of code

It isn't that much extra work. Just selecting the lines and having the comment added by the editor.
--
"Suravye ninto manshima taishite (Peace favor your sword)" (Shienaran salute)
"Life is a dream from which we all must wake before we can dream again" (Amys, Aiel Wise One)

Paulo Caetano
Wednesday, July 02, 2003

I'll echo a few posts because I certainly see this pattern.  The majority of the bad programmers that I know also have the habit of ignoring code spacing.  It's not a rule, nor is it particularly useful, but it can be a hint.

How's about this one: Bad programmers do not, by habit, replace the paper rolls in the bathroom when they finish a roll.  (I'm being half-serious here.)  It's an indication that they do not consider the consequences of their actions, nor do they try to thoroughly complete what they started.

(If I was a better writer, the above would have been hilarious instead of anal.)

1/4 Ain't Bad
Wednesday, July 02, 2003

I'm with ya, 1/4th.

I should add that REALLY bad programmers mount the toilet paper roll so that it feeds off the bottom instead of off the top.

Nick
Wednesday, July 02, 2003

Corn cobs, anyone?

Bored Bystander
Wednesday, July 02, 2003

It sounds funny, but I can see some logic in that. I feel the same way about people who don't start a new pot of coffee after they've emptied the pot (on the machines where I am, it is literally a 7 second 'job', but it constantly goes unfilled). I feel confident in my belief that the person(s) responsible take a sneaking look around and then spirit away, fully aware of their actions. It sounds like something trivial that is nothing more than filling a pot of coffee, but to me it hints at some very insidious, unpleasant personality traits. So the non coffee starters=incosiderate jerks.

On the other end of the coffee maker analysis scale are those people who take a cup of coffee as it just starts brewing. For those who don't make coffee, it should be explained that coffee extraction is not a linear process, but rather about 80% of the flavour comes out in the first 10% of water (these numbers aren't scientific), and then the next 90% is getting tailing flavours...hindcoffee if you will. When someone grabs that first 10%, what is left tastes similar to water which was used to extinguish a cigarette, and to make it worse the culprit inevitably will be dumping their ultra-coffee (unless they're intentionally looking for a bowel cleansing). I peg these people as the people who stumble through life never logically considering the reaction to their actions.

Much can be learned at a coffee pot.

Dennis Forbes
Wednesday, July 02, 2003

Back in Turbo Pascal, we used to use { } for comments, and (* *) for commenting out.  This worked great.  In C I just use #ifdef

For me, the advantage of putting some kind of marker on the left for "big" intent comments, is you can easily spot them and scan them without reading all the detailed code and minor comments.

S. Tanna
Wednesday, July 02, 2003

Just to be a devil's advocate, doesn't the need to create "section-level" comments, then sub-comments, indicate the routine is not cohesive, and should be split into multiple, smaller routines?

In other words, if a routine is cohesive, it will do one thing and only one thing.  If it does one thing, it should involve several steps, all at the same "hierarchical level."

If I was performing a code review and I saw a routine with 3 big steps (with "section-level comments") all consisting of 5 smaller steps (each with a comment), this would indicate to me the routine is not cohesive, and should be split into three separate, cohesive routines. Each of these routines would perform one of the three "big" steps.

I must admit, I used to find the need to create "section-level" comments as well as step-by-step comments, but then I read Code Complete, started building more cohesive routines, and now I don't find I have the need to do this any longer.

Dave
Wednesday, July 02, 2003

re comments about indentation/brace positioning

since the actual position is generally irrelevant to the compiler - shouldn't these things simply be up to the individual programmers editor? (difficult to do in practice mind...)

maybe we need a markup language for code that can be rendered according to the individual developers desires ...

Pete
Wednesday, July 02, 2003

Walter, that seems a pretty good indicator to me. I think it serves to distinguish a deeper difference - between those who take pride in their work and want it to work properly, and those who are accustomed to just bashing through stuff till it works.

The latter approach tends to be found in web firms and consulting firms not led by software engineers. I think it's also characteristic of the pair programming approach.

The former approach can cause conflict in poor environments, for all concerned, because the programmer will not be happy to apply band-aids to code that's otherwise scrappy.

.
Wednesday, July 02, 2003

Dennis, I hear you. Which is why I'm really happy we finally got an automatic coffe machine at work... just press a button and the cup is filled. :-) Previously, at our floor, we were maybe 4-6 guys who made coffee (out of 30 who drank it), and we were quite annoyed in the end (you also had the people who spilled coffee and didn't clean up)... Then again, aren't programmers supposed to be lazy?

BC
Wednesday, July 02, 2003

Pete, I agree a function should implement one clearly defined goal. I do not agree that that means a function has 1 major step.

For example, many functions have 3 major steps (allocate, do it, free) which may themselves involve multiple sub-steps some of which need commenting. 

or

Consider a function to implement line drawing (for example). A bresenham implementation may look like Line function from BRESLINE.PAS at http://www.gamedev.net/reference/articles/article767.asp

However  a real world implementation might have few preliminary steps. So the overall thing ends up something like:-

1. Parameter validation
- possibly several substeps some of which may need comments

2. Check if line eliminated by clipping (major step)
- completely outside clipping area? (sub step)

3. Check for lines which are dots
- is line is just a single pixel (x1==x2) and (y1==y2).  If yes, draw pixel (substep)

4. Check for lines which can be drawn fast using external functions/hardware
- if y1==y2, call draw horizontal line fast function (sub step)
- if x1==x2, call draw vertical line fast function (sub step)

5. Otherwise draw line using bresenham algorithm (similar to BRESLINE.PAS)
- several sub steps, basically each commented section in the example code is a sub-step consisting of several lines.

Now you could argue that the code in 5 should itself be a separate function, but I think this is highly debatable whether it's actually better or merely rigid application of a rule.  If you really think 5 should be a separate implement_bresenhamline type function, I'd question
- is it really more understandable?
- you have a dangerous function (implement_bresenhamline) which doesn't handle the special cases well (or even at all), waiting to be called by somebody from the wrong place in future
- and despite making section 5 a call to a single function, it still doesn't fundamentally alter the function that the main Line( ... ) function, still has 5 major steps - it's just that the last step is one line.

S. Tanna
Wednesday, July 02, 2003

"Is it really more understandable?"

I'd say so.  I hate scrolling.  For me, once a function gets above about 200 lines, it starts to smell.  It's trying to be all things to all people.  I can't understand it all in a glance; there are pages between when a variable is set and when it is used, and I start to lose confidence that if I make a minor change in one place, it won't have some strange side effect somewhere else in the function.

It's like Visual Studio.NET's code-collapsing feature.  If a block of 50 lines of code can be expressed concisely as "FrobnicateWidget()", I'll break it out into a single-use subroutine.  That way when I read the code later on, I don't have to read 50 lines of code to figure out that it frobnicates the widget -- and it's better than a comment, because I don't have to scroll down 50 lines to find the end bracket, losing my train of thought in the process.

"You have a dangerous function (implement_bresenhamline) which doesn't handle the special cases well (or even at all), waiting to be called by somebody from the wrong place in future."

There are some remedies to this.  If you're in C++, make it a private or protected method; if in C, don't include it in your header file.  That way it will only have module or class visibility, and if your modules are kept to a reasonable size, there shouldn't be any confusion.  You can use a naming convention to highlight which functions are single-purpose subroutines, and a comment at the top of each function will give the reader an idea of what the function does, and who calls it.

Alyosha`
Wednesday, July 02, 2003

To answer the 2nd part first: If it's private/protected/local to a C file, another member of the class etc. could incorrectly call it. The usually won't happen now - but could happen when maintained years down the road.

As regards the line example, the bresenham part itself is about 70 lines in the example I gave including comments. and could (should) be shortened. The other bits might add up to 20-40 lines (say), so we're still well under your 200 line limit.

To be fair whether it was 50, 100 or 200 lines, doesn't matter to the point I'm trying to make - which is simply - that it is logical to subdivide non-trivial tasks into the main steps.

If you read some web page or report for example - it's much easier to read the whole, if each section has a sub-heading. Moreover even if some sections need and footnotes for explanation (and they might if the report was a complex as some ideas embodied in code), that wouldn't eliminate the need for
(a) sub-headings to identify the main sections
(b) sub-headings to be clearly differentiated from footnotes, so you are not distracted by one, when reading the other

S. Tanna
Wednesday, July 02, 2003

To get back to the original topic of this thread - using a simple criteria such as poor indentation to identify lacking programmers...

The subject reminds me of the guy who posted here a few weeks ago who vets candidates by having them identify a few obvious syntax and gross usage errors in a hard copy printout of sample code in the computer language of interest.

Much calumny was heaped on him for daring to have such a trivial, demeaning, unprofessional seeming test, but the only apparent purpose was to weed out the "complete" poseurs who had little to no applied experience in the language. IE, those remaining being worth interviewing.

I like simple, elegant solutions like these...

Bored Bystander
Wednesday, July 02, 2003

How about:

  //[ [coder_name] Text
  code
  ...
  code
  //]

and
  // [coder1] TODO[coder2]: Text

First one for long parts of code.

And... I thinks that work of programmer is to make decisions - which type to use, which algorithm to select, need he check for error or not.
  This sort of decisions are too small to be documented or even commented. It is just "all do that way" thing. But it could differ between porgrammers.
So good one is the person, who knows several ways of doing things and selects the best for the task. Father more he/she will always check for boundary condition for every datatype he will use (especiall thouse from external world).

And the _Talent_ is - to keep all that in mind and just write code. Same way as you write plain text in your own natural language without spell checking :)
BTW. proper indentations made by hand is a way to see bad prgrammer. Good one will find an IDE or external tool which sill do this for him just to save time ;)

Nekto
Thursday, July 03, 2003

*  Recent Topics

*  Fog Creek Home