5 ms·
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
by johan_larson 10y ago
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
- TickleSteve 10y agoI completely agree (tho you're getting downvoted by others). Linus's code here is 'clever'... but not good, simple code.
- kwhitefoot 10y agoIt'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
- deleted 10y ago[deleted]
- worldsayshi 10y agoWhile 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.
- apk17 10y agoActually, 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.'
- kwhitefoot 10y agoPascal has references and pointers too so why would they not be able to do it in a very similar way?
- apk17 10y agoIn 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.
- kwhitefoot 10y agoI'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_removal 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_removal#.7B.7Bheader.7CVisual_Basic_.NET.7D.7D http://rosettacode.org/wiki/Singly-linked_list/Element_remov...
- WillAbides 10y agoTaste 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.
- sidlls 10y agoI 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.
- apk17 10y ago'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.)
- sidlls 10y agoI 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.
- MyNameIsFred 10y agoDo 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.
- hairy_man674 10y agoIn 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.