Hacker Newsnew | past | comments | ask | show | jobs | submitlogin

I don't see how Linus' version is better or more readable. By adding an indirection you have the equivalent of a dummy node, both ways are textbook examples of singly linked list deletion.

Note that you save no memory by not naming the implicit (in this implementation) dummy node. A pointer to a pointer remains a pointer to a pointer whether you name the intermediate pointer or not. Personally I think it's more readable to name it. Also, I prefer curr != NULL to simply putting *curr in a condition context. The compiler is going to do exactly the same.

I don't favour Linus' version because it's not readable. Most people who haven't seen this before will need to take some time to check this code. Out of the three basic ways of doing this I'd place this one dead last.



This is exactly what Linus was complaining about.

Your argument is like trying to claim that twenty lines of string munging is more readable than a straightforward regex. That's only true if you don't really get regular expressions.

If you understood pointers well enough, you would see Linus's version as more readable. The other one has so much more unnecessary fluff that makes it extra complicated and less readable. If you don't understand pointers well enough, and "haven't seen this before", of course you'll find it confusing.

Readability is in the eye of the beholder.


"If you understood pointers well enough, you would see Linus's version as more readable"

Sounds like an argument for not using C in the first place. If you need to understand pointers so well that the code presented in this article looks readable just to write a clean implementation of remove_if, your language is seriously lacking in expressiveness.

Data structures are best reasoned about abstractly. A linked list is either null or a pair [data, next] where next is a linked list. remove_if(list, fn) returns null if list is null; if list is [data, next], remove_if returns next if fn(data) is true, otherwise [data, remove_if(next)]. These definitions are much easier to understand than code with two levels of indirection; they are also easily restated as code, without requiring so many levels of pointers.

To put it another way, the shorter the transition between a natural language definition of a data structure/operation and the source code, the better. Since it is almost never the case that a natural definition will involve pointers, pointers should be used sparingly in an implementation. Pointers are unintuitive, easy to make mistakes with, and have been the source of most of the serious bugs we have dealt with over the past few decades; when I see the Linux kernel panic, it is usually because of a problem with a pointer.

I would rather see a few extra lines of code that show that a programmer took the time to think about what they were doing than a "clever trick" that is hard to reason about and which is probably hiding some subtle bug.


If you understood pointers well enough, you would see Linus's version as more readable.

I agree - and as someone who makes his living as a C programmer, I'm sure someone could easily come up with a terse piece of Ruby code that I would view as too clever, and therefore confusing, but which the vast majority of programmers on here would look at and think, "a beginner may be confused by this, but a competent Ruby programmer would not be."

I know some of this comes down to stylistic preferences, and likely some have had bad experiences maintaining code that went overboard with this sort of thing. But I also think that if these snippets of code were being discussed in a forum with a bias towards low-level development, rather than HN, which seems biased towards web development, you'd be seeing some very different responses.


That's the Perl school of thought that basically finished Perl as a language for big projects.

Personally, I have 20 years of C programming experience and I prefer to avoid clever tricks when they add no effective advantage. Making code shorter is not the end-all of programming.

Code is written for people to understand rather than for computers to execute (quoting Sussman). It's fine to strive for efficiency since we're talking about OS development here, but it's not the case in this bit of code really, is it?

What is more likely to confuse a maintainer, an extra level of indirection or an extra trivial "if"? However if you are committed to do all the maintenance yourself, then you can pick as you wish.


That's the Perl school of thought that basically finished Perl as a language for big projects.

It's a great point, and I hated Perl for this very reason. And I'm sure some of it is my defensiveness since this is how I program. But some of it is also my concern that it might signal, during an interview say, that the candidate might not really have a good grasp of pointers. For instance, given the following snippet of code, candidates that didn't really seem to grok pointers wouldn't see that the append() function could not be modifying the list in main():

  struct node {
    int val;
    struct node *next;
  };

  static void append(struct node *list, int val);

  int main(int argc, char **argv)
  {
    struct node *list = NULL;

    append(list, 4);
    ...
  }
And my preference for fixing such an implementation would be to change append to take a pointer-to-a-pointer:

  static void append(struct node **list,  int val);
with an implementation like:

  static void
  append(struct node **list, int val)
  {
    struct node **ppn, *pn;

    if ( (pn = malloc(sizeof *pn)) == NULL) {
        perror("malloc");
        exit(1);
    }

    pn->val = val;
    pn->next = NULL;

    for (ppn = list; *ppn; ppn = &(*ppn)->next)
        /* find tail */;

    *ppn = pn;
  }
and update the call in main() to append(&list, 4).

But as I said, you make a good point, and I agree that your method is the safer of the two options from a maintainability point of view.


Both are relatively trivial. Trading an if for an indirection is not necessarily "better", it's an option.

As I said before, both ways are textbook basic singly linked list deletions. Linus' version does the dummy pointer implicitly. Generally doing things like that requires an extra moment to realise what's going on.

The smartass idea that "anything that is understandable is equally understandable" simply falls flat in the face of reality.

To claim one piece of code to be better than another you need a more scientific argument than "avoiding an if" if you add something else (an indirection) instead.


Just to step back for a moment, it's interesting that /include/linux/list.h in the Linux kernel is a bog-standard linked list implementation. There is nothing particularly clever about it.

Speaking of the word clever, I think some are clutching that a little too proudly. In this context it doesn't mean "more skilled" or "more efficient" or "technically better", it simply means doing something in a particular way because you think it is unique. There is little to no argument to be made in favour of the approach in the linked blog entry, but there are many potential risks with it, not least of which is that it adds one more potential misunderstanding by skilled practitioners. While I love pointers to bits, and love unmanaged, native code, there is a good reason why most environments now trying to reduce their usage, even with the bestest of the bestest developers manning the keyboards.


Obviously using safer languages makes sense where they're available, but if someone's going to use pointers I'd prefer they understand them. My argument in favour of the blog entry's approach is "It's easier to read [for me]".

The more variables and control branches I need to track, the harder it is to verify the code in my head. Obviously, there is such a thing as "clever" code, but I don't think this is it.




Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: