Signs your code sucks | Hacker News Reader

Signs your code sucks

turnleafdesign.com

10 pointsfogus17 comments

I don't like it when people treat #1 as an unbreakable rule. Sometimes it takes more than 25 lines of code for a function to complete it's one task. The book "Code Complete" talks about this and has studies to back it up.

Here's the quote from the book:

From time to time, a complex algorithm will lead to a longer routine, and in those circumstances, the routine should be allowed to grow organically up to 100-200 lines. (A line is a noncomment, nonblank line of source code.) Decades of evidence say that routines of such length are no more error prone than shorter routines. Let issues such as depth of nesting, number of variables, and other complexity-related considerations dictate the length of the routine rather than imposing a length restriction per se.

If you want to write routines longer than about 200 lines, be careful. None of the studies that reported decreased cost, decreased error rates, or both with larger routines distinguished among sizes larger than 200 lines, and you’re bound to run into an upper limit of understandability as you pass 200 lines of code.

And often a single routine is a sequence of--very many--but linear steps, one after another.

Splitting the routine into 20 functions, one for each step, doesn't necessarily make it any more understandable: it reminds me of a DailyWTF in which a programmer was told to split a large routine into functions and just did something like this:

    doPart1()
    doPart2()
    doPart3()
    ...

It's just as effective, if not better, to comment what each section of the large routine does. One advantage of keeping it in one routine is if there are dozens of values which are global to the routine and are not changed throughout it: it would create an ugly mess to pass those all around to each of the subroutines.

Though, when you can, you should split up those long linear routines. Especially when there are some variables global to the routine, but not all. The passing makes the shared variables explicit.

That violates the DRY (don't repeat yourself) principle. Repeatedly entering the same parameter declarations is busy-work, wasting labor and valuable screen space.

A better way is to use nested functions. Long-lived state goes in the outer function's variables, so you do not have to repeatedly declare them. Ephemeral variables go in the inner functions. Sub-tasks get encapsulated with zero boilerplate.

If your language does not have nested functions, use a worker class. The algorithm-wide state goes in member variables, initialized from constructor parameters. The sub-tasks are methods. A benefit is that sub-tasks from different jobs can be batched to better use the instruction cache and branch prediction information: a.do_step_1(); b.do_step_1(); a.do_step_2(); b.do_step_2(); ...

Yes, nested functions are probably good for this. (I was about to suggest use of the reader monad in the original comment, but then left out this suggestion for being to arcane (and did not include the nested function suggestion either).)

Nested functions are good for this kind of code, even when they are only used once and do not abstract --- because you have less variables in the innermost scope. (And when you are going to mutate variables at all, then strongly prefer to do it to variables in the innermost scope.)

> You need to use comments to explain the code – Code should be able to explain itself and should be in a format that is easily readable.

No thanks. Has this author ever written any algorithm more complicated than a basic CRUD app? I'd like to see him read some kind of well written complicated graphics algorithm without any comments. The "why" is often really really important, and you can't read that from the instructions.

Are little ASCII comments mixed in with code the best way to explain a complicated algorithm?

Algorithm, no. Details of the implementation, yes. Some implementations require subtle things that aren't obvious unless you're mired in the details. It's nice to have reminders why, say, this particular value needs a +1.

Agreed. Moreover, certain "why"s like "why's this here instead of over in this class?" or "why is this bit of code repeated?" can't necessarily be explained by refactoring.

And sometimes there's just certain technical limits you just can't get around, and you have to comment those.

Your website says "Error establishing a database connection" when people visit it?

Ironic, no?

> You need to use comments to explain the code – Code should be able to explain itself and should be in a format that is easily readable. If you find yourself needing to explain what your code is doing then you may want to look into rewriting that code. EDIT: This is not referring to using comments (e.g. javadoc) to explain the purpose of a method/class and its inputs and outputs.

-- What? --

This is only true of "unclever" code. You haven't written a lot of code if you haven't found a concise, optimized, abstract way to solve problems that aren't intuitive when first seen with the naked eye.

We aren't writing books, we're writing instructions for a computer and those two things can sometimes be very, very different.

Oh dear god, that nonsense again. That might be true when you write boring, straightforward business code. But when you implement a complex algorithm, that's just silly.

It's also silly if, for example, you need to do "clever" things for performance reasons. Any comment that explains the why behind your code is a good comment.

See the recently discussed square-root approximation algorithm as an example.

Just a reminder in the future to delete the number from these posts, to remove the "appeal to lists" factor-- as per the site guidelines-- so that it becomes "Signs that your code sucks" as opposed to an amusing list.

I am not so keen on "absolutes". There is no right or wrong way, there is what works for you in a specific context. I find that most people who deal with "strict rules" haven't spent enough time writing code to have an opinion about it.

Reply on news.ycombinator.com