Hacker News Reader

Comment by johan_larson | Hacker News Reader

For my money, Linus's example of "good taste" gives up rather a lot of clarity to achieve succinctness. The original is simple and clear. His preferred version is shorter, but also harder to understand because of its use of a complicated indirection. And that's not good taste. It's just showing off.

“Programs must be written for people to read, and only incidentally for machines to execute.” ― Harold Abelson, Structure and Interpretation of Computer Programs

View on HN

Do you do much C programming?

If I put my "Java goggles" on, the first, "untasty" snippet looks so very obviously to be the right way to do it. But...

If I put my "C goggles" on, the "clever" version from Linus looks much more direct, obvious, and better in general...much like the final example from the article author.

While I agree, I also think that it depends on your background. I guess pointer arithmetic should be second nature for a kernel developer? So there might not be much clarity cost then.

I completely agree (tho you're getting downvoted by others).

Linus's code here is 'clever'... but not good, simple code.

It's a long time since I wrote much C but Linus' version seems like idiomatic C to me. The use of pointers in C is an ordinary thing and those who write a lot of C should be fluent in their use.

It's interesting to apply the same technique to other languages. I have to use VB.net most of the time so here are implementations of the tasteless and tasteful versions in VB (untested so there might be bugs). Even in VB the tasteful version is shorter and, I think, clearer.

    Module Module1

      Public Class ListEntry
        Public value As String
        Public [next] As ListEntry
      End Class

      Public Head As ListEntry

      ''' <summary>
      ''' Straight translation of Torvalds' tasteless version.
      ''' </summary>
      ''' <param name="entry"></param>
      Sub RemoveListEntry(entry As ListEntry)

        Dim prev As ListEntry = Nothing
        Dim walk = Head

        ' Walk the list
        While walk IsNot entry
          prev = walk
          walk = walk.next
        End While

        ' Remove the entry by updating the head or the previous entry.
        If prev Is Nothing Then
          Head = entry.next
        Else
          prev.next = entry.next
        End If
      End Sub

      ''' <summary>
      ''' Straight translation of Torvalds' tasteful version.
      ''' </summary>
      ''' <param name="entry"></param>
      Sub RemoveListEntry1(entry As ListEntry)

        Dim indirect = New ListEntry
        indirect.next = Head

        ' Walk the list looking for the thing that points at the thing that we
        ' want to remove.
        While indirect.next IsNot entry
          indirect = indirect.next
        End While

        ' ... and just remove it.
        indirect.next = entry.next

      End Sub

End Module

Actually, I find the double pointer version easier to understand, and incidentally it is also the way always wrote this in C. And I pitied the pascal programmers who had to use the original version.

'Simplify so a fool can understand your code, and you will have fools editing it.'

Pascal has references and pointers too so why would they not be able to do it in a very similar way?

In the original pascal pointers/refs came only out of new (AFAIR), you couldn't point into structures or to local variables. That makes this trick impossible.

'Modern' 'pascals' were different.

I'll grant that you can't do it quite as efficiently as in C but you can do the same trick as I showed for VB.Net (in another post in this thread) and create a new entry that points at the head then use that as your indirect pointer. The problem is caused not by being unable to point at locals or members but by Pascal's pointers being strongly typed so that you can't simply dereference some random word length memory cell to get a new reference.

So not identical but still simpler than the tasteless version.

I thought that this was such a fundamental idea that it should be on Rosetta Code but the page for it at http://rosettacode.org/wiki/Singly-linked_list/Element_remov... is empty.

Perhaps we should all rush over there and fill it in. :-)

Edit: See entry on RC at http://rosettacode.org/wiki/Singly-linked_list/Element_remov...

Taste is dependent on the audience. An audience of kernel maintainers would probably share Linus' taste here, while an audience of enterprise developers would find the original code more palatable.

I think the specific example can create a distraction because of the use of indirection, but that there is a more general point to discuss: at what point is a chunk of code "too clever?"

One argument is that code should be written so that junior or average (or even below average) developers can be put to work on it. Another view is that part of a junior developer's learning experience ought to include not shielding him from alleged complexity or "clever" constructs, because sometimes they really are better or even unavoidable.

Cleverness for its own sake is one thing to avoid, but I think that's almost too subjective a standard to use.

'cleverness for its own sake' often makes it into standard practice later.

And I seriously fail to see the 'for its own sake' here. This cleverness reduces the number of moving parts, does the 'dont repeat yourself' and thus reduces ways modifications of the code could break it due to overseeing a case.

I just shuddered to think how this would be done in java - with a LinkUpdater interface, and a RootLinkUpdater and ElementLinkUpdater, and one instantiation per iteration, and suddenly we have a lot of code around to keep the active code's function simple - to a point where it is completely pointless because of adding a lot of machinery. (And java does collections differently anyway.)

I agree with you, about this specific example and in general.

When I look back on my career, often when I've said something is "clever for its own sake" was only clever because of a lack of understanding on my part, and rarely for its own sake, outside of toys and obscurity competition where such cleverness is the point.

In userspace this is generally true, and it sucks because "good taste" should really be an engineering determination not compromise to human laziness and mediocre slackers.

Reply on news.ycombinator.com