25 ms·

Applying the Linus Torvalds “Good Taste” Coding Requirement

• PJDK 10y ago
  I was given a piece of advice very early on in my career that I've always been grateful for, which is fundamentally the same as this. IF and FOR are both code smells. One case of this is just simplifying loops with some functional goodness var listOfGoodFoos = new List<Foo>(); for(var i = 0; i< listOfAllFoos.Count; i++) { if(listOfAllFoos[i].IsGood) listOfGoodFoos.Add(listOfAllFoos[i]); } return listOfGoodFoos; VS return listOfAllFoos.Where(x => x.IsGood); But perhaps a more interesting point is it can also be a a sign of DRY gone wrong - two things that aren't actual repeats of each other, but just similar code, are smushed together with lots of IFs hang around to make it actually work. A connected piece of advice was "it is easier to push things together than pull them apart" so err on the side of assuming two bits of code are not the same, knowing you can refactor them together later (probably only a few hours later) if it turns out they are in fact pretty similar.
  • yitchelle 10y ago
    Does it come down to having a deeper understanding of what is available? In your example, I guess a new engineer may not know that Where() is available, but a seasoned engineer should know. I also think that this why the social aspect of code review is so important. It is a knowledge sharing mechanism. edit: grammar
    • flomo 10y ago
      > I guess a new engineer may not know that Where() is avaiable, but a seasoned engineer should know I'd guess the opposite. Its only in the last few years that many mainstream languages have adopted this sort of functional syntax. The new engineer probably learned it right away from the docs, while the seasoned guy churns out smelly-old for loops without a second thought. GP makes a great point about different loop logic being 'smushed together', that seems to be the most common kind of ugly code (and I've been guilty of it). Perhaps that is psychological. for/while makes loops appear expensive, while the functional code hides that detail and the programmer doesn't think of it.
      • __jal 10y ago
        > I'd guess the opposite. Might depend on how trendy the new feature is. Because I still see newish Java programmers who apparently don't know that java.date.util exists, and who also don't know that rolling your own date math is only done by date manipulation library authors and connoisseurs of slow moving train wrecks. A second, related but harder to pin down, issue exists here. I see relatively inexperienced developers who, for instance, get a little confidence with Javascript chaining and callbacks, and work themselves into corners where a given filter method (like the 'Where()' example) doesn't quite do what they need, and suddenly they have no idea what to do. So they hack in something awful in the middle, forget that it is a weird homegrown something instead of a library function, and see profoundly strange problems later. Both of those are things programmers grow out of, hopefully. But they seem to be instances of having APIs with large surface areas causing what amounts to research failures.
    • PJDK 10y ago
      Absolutely - it's why the advice was so good as it led me to trying to discover alternative patterns that remove the ifs. On our team I try and code reviews to both be about code quality and learning. Reasonably often we'll go down mini rabbit holes about different ways things could be written and the various merits of them. We also try and have more than two people reviewing to spread the learning even more. That can make it worthwhile rewriting good sections of code to be worse just to discuss why the original way was better. As a junior I learnt a great deal from reviewing the senior devs' code.
    • prewett 10y ago
      In case any other seasoned C++ engineers are worried that they have missed something big in all the new C++ specs, the above code seems to be C#.
      • PJDK 10y ago
        It is, sorry,I should have specified. Also I don't think many people would use a loop like that when foreach is available.
        • krylon 10y ago
          Having made my fair share (if not more) of terrible (and terribly embarrassing) errors in C using the for-loop, I love foreach and its brethren in other programming languages. I only write classical C-style for loops very, very rarely these days.
        • optionalparens 10y ago
          For loops, even in C# are used in performance sensitive contexts. I believe things have changed in versions of .NET and if you're using things like Mono that also have its own versions. Foreach generally (or used to) generate more garbage/allocations. Foreach against some types in some versions of C# (ex: structs) produces no allocations. There are also some ugly mutation related things you could do using for instead of foreach, but I'd call that bad code. The same things are true on many platforms - foreach is generally preferable for non-peformance sensitive code. In some languages and libraries, foreach constructs will do things similar to auto pointers or reference counted pointers, which makes them safe enough, but also slower. The point is that foreach can take up more memory, cause garbage, or cause allocations/free on some platforms and languages which is not always desirable. For is therefore a better choice if you don't want this behavior. Mostly though, using for instead of foreach is micro-optimizing in these cases. Where I'd usually do it is somewhere critical in my code, like something called a lot per frame game loop for example. If I caught someone using a foreach that generated lots of allocations and is called a lot of times in these contexts, I'd probably kill them (and have). So in short, I guess I'd agree that "many" people would do it, but it's context dependent which include performance goals, target platform, language limitations, and more. Mostly, it's better to just write the code that is safe and works, and go back and fix any bad decisions like this. If it's obvious though, I don't have a problem with the optimization from beginning.
  • sharkjacobs 10y ago
    I use eight space tabs when I write C, to force myself to think twice about using if, for, and nested loops (I convert it back to 4 space tabs before I commit anything)
    • solipsism 10y ago
      Does your coding style not involved lining function arguments (and other things) that are broken into two lines up vertically? Converting between 8 spaces and 4 would screw this up.
      • clusmore 10y ago
        My preferred whitespace code-style is to use tabs for indentation and spaces for alignment. Under no circumstances should two consecutive lines be aligning something (with spaces) and have different levels of indentation (with tabs), I can't imagine why you would ever need to do this, so this has never been an issue for me.
        • solipsism 10y ago
          My preferred whitespace code-style is to use tabs for indentation What does "I convert it back to 4 space tabs before I commit anything" mean, then? If you're putting \t tabs in your file, 4 space tabs vs 8 space tabs refers to the way your editor renders \t. What are you changing before committing, your editor settings? Edit: oops, just realized you're not the person I was responding to. The convert it back before I commit anything comment suggests he/she is using spaces. Or is very confused.
          • clusmore 10y ago
            Sorry, I should have mentioned that I was not the GP. Even if you use spaces rather than tabs, the idea is the same. Do you mean something like the below? Note: \t represents tabbing (not necessarily the number of tabs, just spacing using tabs), and _ represents spaces. int f(int x, \t\t__int y) // one 4-width tab, pad with spaces { \t\t// do stuff; } int f(int x, \t\t\t\tint y) // one 8-width tab, whoops { \t\t\t\t// do stuff; } In this case, you should not be using tabs, since logically you haven't entered a new block yet and as such should not be increasing the indentation. It should be int f(int x ______int y) { \t\t// do stuff; }
  • falcor84 10y ago
    I definitely agree with this, but note that the semantics in your code are different from those in Linus's example - your code filters out ALL matches.
    • fdgdasfadsf 10y ago
      This is actually an important point. Quite a few times I've written something that started as a list comprehension, changed it to a lambda later in the sprint and finally to a loop when the requirements became more detailed. - The real world is unfortunately full of business requirements that think that they are special flowers.
    • PJDK 10y ago
      Indeed, I should have been clearer this is just a similar example, not equivalent code (and I assume nothing like performant enough for Linus's needs)
  • legulere 10y ago
    That is more imperative programming (your first example) vs. functional programming (your second example). The problem with functional programming is that you need to know how the functions are implemented to be able to estimate performance.
    • AnimalMuppet 10y ago
      Especially for something like kernel code, which was the original example. "Obvious cost" matters at least as much as "obvious code" in that world.
  • twwl 10y ago
    It's not simplifying it though, it's just hiding the complexity in syntactic sugar. I prefer the first way of doing things vastly over the second. Yeah, it's more code, but it's also more or less "what is actually happening", instead of an euphemism which has to be unpacked.
    • sidlls 10y ago
      At some point that breaks down though doesn't it. I mean from one perspective a conditional in high level code doesn't describe "what is actually happening, either." That's especially true if an optimizing compiler or interpreter mangled it up. From another perspective your argument can be applied to any function call in library code. In either case I don't think your position is that strong.
      • twwl 10y ago
        > I mean from one perspective a conditional in high level code doesn't describe "what is actually happening, either." Sure, and from another perspective even the most efficient code does nothing to stop the heat death of the universe and is therefore functionally equivalent to doing anything else or nothing at all. However, that's just splitting hairs. It's not really an argument anyway, but rather a preference.
        • Dylan16807 10y ago
          The question is more whether it's a reasonable preference to think syntax X is helpful but syntax Y is hiding things. Thinking deeper might show that the issue is actually familiarity, and the two should be rated the same in terms of abstraction.
    • lmm 10y ago
      All programming languages are "euphemisms" in that sense - even C has a lot of layers between it and how modern hardware behaves. It is simplifying if it expresses the important aspects of the result rather than the implementation details of how the machine calculates it.
  • sidlls 10y ago
    Well the second snippet hides the loop and conditional which surely are implemented in "Where". Is that a code smell by proxy?
    • krylon 10y ago
      In a way, yes. On the other hand, it boils down the code that some poor soul of a maintenance programmer needs to deal with to a single line which is more readable, too. Win-Win.
      • sidlls 10y ago
        I don't disagree. But then again I've had peers review code similar to this and flag an issue saying it's too clever because that poor soul might not know what "where" is doing or that the use of lambdas is "too complex."
        • PJDK 10y ago
          That is certainly something I've also come across. This kind of functional style is very readable once you understand it, but it does have a learning curve just like learning what "for(var i = 0; i< length; i++)" means. I actually did a little presentation to the team about how to refactor a for loop to an self implemented where/filter function to explain the connection. For me, realising that all the where was doing (or at least potentially, there might be some behind the scenes optimisations going on) was passing in the condition for an IF in a FOR loop suddenly clarified everything for me. In case anyone's interested, this is these are the code steps (javascript) var people = [{name: 'bob', gender: 'male'}, {name: 'dave', gender: 'male'}, {name: 'sarah', gender: 'female'}]; var getAllMen = function(population) { var returnList = []; for(var i = 0; i< population.length; i++){ if(population[i].gender === 'male'){ returnList.push(population[i]); } } return returnList; } var males = getAllMen(population); ___ var getAllMen = function(population) { var returnList = []; for(var i = 0; i< population.length; i++){ if(isMan(population[i]){ returnList.push(population[i]); } } return returnList; } var isMan = function(person){ return person.gender === 'male'; } var males = getAllMen(population); ___ var filter = function(anyOldList, filterCondition){ var returnList = []; for(var i = 0; i< anyOldList.length; i++){ if(filterCondition(anyOldList[i]){ returnList.push(anyOldList[i]); } } return returnList; } var isMan = function(person){ return person.gender === 'male'; } var males = filter(population, isMan); var females = filter(population, function(person){ person.gender === 'female'; }); var females = filter(population, person => person.gender === 'female'); var females = filter(population, x => x.gender === 'female');
  • optionalparens 10y ago
    Like most things, it's all contextual. Were I your manager and we were working on a business app, this might pass code review just fine or I might even suggest what you did here if you had showed me the original. Another piece of context in a business app might be - "Do we need these values now or later?" That is, should this be lazy, stream results, something like that? I am assuming this is supposed to be C#, but still speaking somewhat generally. You might need to instead yield rather than looping and returning an intermediate list. Better yet, I'd also look for you to return something like a read-only list (ex: IEnuemrable in C#) since this is a query result and that can be done with the first version. Unfortunately if we had been writing a game though lets say, I would have failed you miserably for the latter version and added 1 mental strike to the "send you packing" count. Why? For loops are most certainly not code smells on most platforms. As a few people said, the mechanics of "Where" in some languages may just be cloaking a for loop. More importantly, for loops tend to be much more performant, can be inlined easier, and generally produce less garbage than lambdas and other similar language constructs. It may different than I last looked, but back when I used to touch C#, for was most certainly preferable for anything performance sensitive provided you didn't need laziness, and if you did, there's still at least "yield" (though at that point, we're approaching what Where can does anyway). I could go on about cache lines, trashing, GC pauses, and so on, but hopefully you get the idea. Overall, I am afraid I must disagree and point out that your advice you treasure is too generic. FOR and IF are very valuable in certain contexts. I think the real advice someone might have been trying to give you is to aim for more functional constructs, that is functions that are pure/idempotent/referentially transparent, compose, and operate on sets of data where possible rather than individual points of data. Like any advice, this isn't universal and quickly unravels in many situations such as performance-sensitive contexts like game programming, real-time programming, and so on, or may in fact just be the best practice of the target language.
    • PJDK 10y ago
      I absolutely agree with you about performance stuff, if you are in a position where milliseconds matter this may well not be the right way of doing things. Also, to open up another interesting area, Linq operating on IQueryable objects can do some pretty interesting stuff in the background, merging sql queries and the like. I've never looked into if it does anything similar for IEnumerable - although I will should the situation come up. FOR and IF are indeed valuable, but that doesn't make them not smells. For me a code smell is not an anti-pattern - it is just an indication that something has the potential to be wrong. If you are writing lots of IFs and FORs in your code that might well be fine, but it has a good chance of causing problems. Smells, for me, just mean "justify this decision" - not "never do this".
  • thomasahle 10y ago
    This also comes easy, if you are using a more expressive language: remove e [] = [] remove e (e:xs) = xs remove e (x:xs) = x : remove e xs This even has the benefit of making it very clear whether you've remembered the case where 'x' is not in the list.
    • deleted 10y ago
      [deleted]
  • GrumpyYoungMan 10y ago
    You don't know what that Where() costs you, though. The abstraction in your second snippet does _at least_ as much work as the code in the first snippet _and_ has an unknown level of extra overhead. Abstractions are not free. Because we can almost always afford the luxury of abstractions, some people forget that there's an engineering tradeoff there. There's always something underneath that nice, pretty-seeming abstraction that has to do the dirty work. Because that abstraction has to be pretty and handle generic situations, it (not always but often) can't be optimized to be more efficient in specific situations.
    • ehsanu1 10y ago
      Zero-cost abstractions are a thing though. Compiler magic can even fuse together multiple calls like this on the same collection into the equivalent regular loop. So it can in fact be faster rather than slower, in the right context.
  • posterboy 10y ago
    Don't write the block to the if without curly braces on the next line. It's a common source of bugs when the code has to be extended (possibly by someone else), to forget to add the braces.
• smnscu 10y ago
  I think "competitive" (i.e. solving algorithmic challenges for fun) coding really gives you some insight into how to write code that's short and to the point. The user with the most reputation on LeetCode, for example, consistently posts solutions that are surprisingly short, efficient, and readable. https://discuss.leetcode.com/user/stefanpochmann https://discuss.leetcode.com/user/stefanpochmann (some random examples) https://discuss.leetcode.com/topic/18731/7-lines-c-c https://discuss.leetcode.com/topic/18731/7-lines-c-c https://discuss.leetcode.com/topic/16988/7-lines-3-easy-solutions https://discuss.leetcode.com/topic/16988/7-lines-3-easy-solu... https://discuss.leetcode.com/topic/33430/6-lines-o-log-min-m-n-ruby https://discuss.leetcode.com/topic/33430/6-lines-o-log-min-m...
  • sandos 10y ago
    The code might be short and to the point, but it definitely is not easy for me to understand! The code feels like a bit like clever perl one-liners.
    • fiveop 10y ago
      Or you could just loop n-1 times and adjust the array indices in the loop body, so that each statement only deals with one corner.
  • ubernostrum 10y ago
    Well, I went over there, looked at the list of recently-active topics, and contributed exactly one boring solution to one of them. Now to never post there again.
  • DeltaWhy 10y ago
    Stefan Pochmann is also the inventor of two different methods for blindfolded Rubik's Cube solving. Classic Pochmann is a quite elegant solution as it effectively deals with one piece at a time, where previous methods generally separated orientation and permutation into separate steps, and deal with more pieces at once. M2 is 'tasty' in a different way - there are many more edge cases to handle but it replaces the 14-move swap sequence in Classic Pochmann with a single move. Perhaps I'm stretching a bit but it seems like an example of the same kind of thinking. Side note - blindfold Rubik's Cube is way easier than you think. If you're a programmer and can already solve one sighted, I'd imagine you could learn Classic Pochmann in a week.
• kccqzy 10y ago
  I would argue for the last example it is better to write split the loop into four. I believe the intention is even clearer (because you are doing four different things), and that it is also faster even when optimizations are off due to better cache behavior. It may also make a smart but not "sufficiently smart" compiler to transform two of the loops into simple memset.
• solipsism 10y ago
  The root of the problem is that "the thing that points to an entity" is not a consistent concept in this kind list. Sometimes it's "head", sometimes it's an entity's ->next. I wonder what Linus would think of implementing the linked list with a dummy head node, having no value and pointing to the first entity. Personally, I think that would be tasteful. It allows for simple loops like the "good taste" one, but you don't have to think so hard about how to implement a simple operation like remove. You don't need the added indirection.
  • dpark 10y ago
    This is exactly what Linus's code does. "indirect" is just "dummy->next" but doesn't pay the extra space cost for the node value. If writing this code in a higher level language, I think the dummy node would be more appropriate and idiomatic despite the slight increase in space.
  • majewsky 10y ago
    It's one more pointer that you have to resolve, everytime you do something with the list. Sounds smelly to me.
    • dpark 10y ago
      Depends on where you store it. If you're just passing around node pointers, then you could create the dummy node on the stack only when you need it and it's the same indirection cost as the "indirect" pointer. If you've got a "list" struct that you're passing around (pretty unlikely in C), you could embed the dummy node directly.
• AceJohnny2 10y ago
  These are great examples, and they hint to, but do not mention, the big counterpoint: development time. In his own examples, the author admitted that though the code was ugly, it worked. He then spent extra time reworking the existing code to make it, well, prettier. "If I had more time, I would have written a shorter letter." -- Voltaire The problem is that, in many (most?) professional settings, the developer is under huge pressure to get shit done. Not get it done perfectly, not done beautifully, but just done in the first place. We rarely have the leeway to spend extra time refining existing code, existing features or bugfixes, to make them prettier. It's gotta be done, and it's gotta be done yesterday because maybe some other part of the project is blocked because of it or some customer paid for it and it was supposed to be done last month or it broke and why the hell isn't it fixed yet!? Some projects, I daresay mostly open-source projects, can afford to be detached from the pressure of deadlines that corporations require, and reject code that isn't to their quality standards. Sadly, that's not the case for most of us.
  • nitrogen 10y ago
    But how much of that deadline pressure arises from mounting technical debt due to skimping on code quality?
  • tarikjn 10y ago
    I think you are overlooking a number of things, first of all the code examples are not just prettier they are faster as they bypass test conditions. Save possible compiler optimization or CPU branching strategies which I am not too familiar with, this would at least still save execution cycles in certain cases/languages. Might seem small here, but when it's for library code, code that gets duplicated in memory and needs to also copy variables and closures (i.e. rows on a webpage), or simply a practice all across the codebase, it will have effect. Not only that, but as the author outlined it's easier to read and parse for another developer, and there are less chances for errors. I, for one, strongly believe that pushing software excellence reduce the number of high pressure situation to "get shit done", as it's better to take control of the code than other way around.
  • flukus 10y ago
    They are taking development time into consideration, development time in the long term.
• no_protocol 10y ago
  > it only performed 256 loop iterations, one for each point along the edge alarm bells There are only 252 points along the edge. This code will act on each corner twice. If you were performing an operation like `+= 1` on each edge element, this code would be wrong. When you copy and paste it later and change all the `= 0` to something else, you might end up with an unfortunate surprise. Once I saw this mistake in the 2nd code example, I guessed it would also appear in the 3rd one. Sure enough, it does. This is just as unsavory to me as the original code. I'm not sure if the author is here or not, but if you are, can you see the fix?
  • elouataoui 10y ago
    The way I'd do it if I didn't want to use ifs is to loop n times, and for each row fill the first cell and the last one. So now I have the left and right side at O(n). Then i'd loop n-1 times starting at the second column and for each column fill the first and last cell again. O(n) all in all without repeating yourself, and no tests. Another way to look at this problem altogether would be to change the data structure, so instead of using a classical 2d array the problem becomes trivial if you use a spiral array. Filling the edge in this case is just a matter of filling the first 4*(n-1) cells. Obviously this solution's acceptability depends entirely on what you're gonna do with your data afterwards.
  • deleted 10y ago
    [deleted]
  • usrusr 10y ago
    Noticed the same thing, but only in the third, cleanest version. No better way to illustrate its merits relative to the initial two versions. Spoiler alert: flipping that single bit from i=0 to i=1 might deserve a little comment of the "why" category.
  • tomerv 10y ago
    Here's my fix: for (i = 0; i < GRID_SIZE - 1; ++i) { // Note the "-1"... // Top Edge grid[0][i] = 0; // Bottom Edge grid[GRID_SIZE - 1][GRID_SIZE - i - 1] = 0; // Reverse the order: from left to right // Left Edge grid[GRID_SIZE - i - 1][0] = 0; // Reverse the order: from bottom to top // Right Edge grid[i][GRID_SIZE - 1] = 0; } This way, for every row/column, the loop acts on all but the last cell, which is actually the first cell of some other column/row.
• akkartik 10y ago
  I dunno, the first example seems unsatisfying. The original has that ugly condition, the "good" version seems overly clever. And for all the talk of taste and aesthetics, both versions ignore an elephant in the room: defensively dealing with `entry` being absent from the list. Not that I've never abused addresses like this. But having written this multiple times, I currently prefer something like this: remove_list_entry(entry) { if (head == entry) { head = head->next; return; } for (prev = head; prev->next; prev = prev->next) { if (prev->next == entry) { prev->next = prev->next->next; return; } } } There's still an `if`, but it isn't so bad since it's an early exit. What is "tasteful" about hiding the fact that one of the cases is far simpler than the other? There's conditionals and then there's conditionals. There's cases and then there's cases. The other benefit of this approach: you eliminate segfaults by construction, because the `while` has been replaced with a much more normal `for` loop iterating through the list. Keeping it normal is 80% of the secret to avoiding security holes. Rather than appealing to something nebulous like "taste", I prefer to focus on concrete functional benefits. What is a situation where someone using or modifying this function might be subtly led astray? That seems a better guide. (Earlier version posted on reddit: https://www.reddit.com/r/programming/comments/59cq8r/applying_the_linus_tarvolds_good_taste_coding/d97pcrl https://www.reddit.com/r/programming/comments/59cq8r/applyin... )
  • dwenzek 10y ago
    > both versions ignore an elephant in the room: defensively dealing with `entry` being absent from the list. Indeed, both versions really have a poor taste. Both are code snippets which can only work under very restrictive assumptions. The main argument `head` is implied and the `entry` to be removed must be present. Such piece of code has to be duplicated and tuned for every list and use case. I even wonder why Linus bothered to show this example, while he leads one of the most famous software masterpiece.
    • mobiuscog 10y ago
      This isn't an API though. 'head' is fine being implied, and if 'entry' doesn't exist, why is it being passed in ? That's a further code 'smell' to follow up. Sure, you need to be massively defensive if you're writing an API or boundary, but within an owned codebase, if you're eliminating edge-cases, you should follow them all the way up. tl;dr if you're calling remove_list_entry, only do so if you have an entry. Or course, I'm not Linus, so I have no idea of his thought process.
• sverrirs 10y ago
  I don't know why I'm even replying, this will get so much hate here, oh well... 1. Just because things aren't done "the way you would do them" doesn't mean they're "bad" or "wrong". 2. If you're not on a solo project, I've found writing correct but less "clever" code to help shorten ramp-up time for new devs and be more beneficial to future maintainability of the code-base and system. TL;DR; Swapping values by XOR'ing may look elite and clever but hurts you in the long run.
  • kazagistar 10y ago
    I am pretty sure it hurts you in the short run too. After all, the microcode probably does a register rename or the like with a normal swap, but has to actually burn cycles on using aritmetic for the xor swap.
    • sverrirs 10y ago
      On modern CPU architectures, the XOR technique can be slower than using a temporary variable to do swapping. One reason is that modern CPUs strive to execute instructions in parallel via instruction pipelines. In the XOR technique, the inputs to each operation depend on the results of the previous operation, so they must be executed in strictly sequential order, negating any benefits of instruction-level parallelism.[3] https://en.wikipedia.org/wiki/XOR_swap_algorithm#Reasons_for_avoidance_in_practice https://en.wikipedia.org/wiki/XOR_swap_algorithm#Reasons_for...
  • TickleSteve 10y ago
    Absolutely. Further, I would consider Linus's one-liner 'indirect=...' to be 'clever' code. If I were reviewing it, I would want it expanded into more obvious code as it requires too much cognitive overhead to parse.
    • sverrirs 10y ago
      Agree on the cognitive overhead. People will and are arguing that "it is kernel code, it should be complicated" but I really think this is backwards. Because it is low level and debugging is really cognitively intense and difficult at this level the code should be even clearer and easier to understand.
• tempestn 10y ago
  Not being used to C syntax, I took this as a typo at first: indirect = &(*indirect)->next; The position of the parentheses make it look like you're dereferencing only (* indirect). But on second look, you need the parens there so that it's (* indirect)->next as opposed to indirect->next. Then the & operates on the whole thing. I'd be tempted to wrap it in a second set of parens for clarity, but perhaps it's entirely obvious to a programmer who's actually used to using pointers. IE: indirect = &((*indirect)->next);
  • TickleSteve 10y ago
    To me, its 'clever' code.... i.e. it should be simplified. This is not good code, it requires too much thought to realise what its doing.
    • tempestn 10y ago
      It's busier to look at due to the syntax of the language, but if you're comfortable with pointers one could argue that it's conceptually simpler than the first example. And since linked lists are all about pointers, operating on the pointer directly seems logical. That said, for someone unused to the syntax (as I am!), I agree that it is definitely harder to mentally parse. So which is objectively better probably depends on your audience.
      • palunon 10y ago
        >So which is objectively better probably depends on your audience. I guess when your audience is kernel developers, this isn't that much of a problem...
        • TickleSteve 10y ago
          not true at all.... kernel developers are not mythical beasts, they're just regular low-level developers. Nothing special. I've got 20 years experience developing this kind of stuff and to me, its not as simple as it could be, therefore it could be better.
• 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 ago
    I completely agree (tho you're getting downvoted by others). Linus's code here is 'clever'... but not good, simple code.
    • kwhitefoot 10y ago
      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
• vacri 10y ago
  Speaking of coding niceties, when I started branching out from bash into a 'real' language, Python, I couldn't find a switch statement. A colleague of mine said that Python didn't have one, and that switch statements were a 'code smell'. I didn't really understand that, and asked about those times that you genuinely could use a switch statement, and the answer was "just use a long if-else function". I can't remember his justifaction for it, so clearly it didn't stick I still don't see the real pragmatic difference between a switch statement and a long if-else function - anyone feel like ELI5'ing it for me? Switch statements just seem more concise and readable to me...
  • somestag 10y ago
    If your switch is more than a few lines, the Pythonic way is to use a dictionary, rather than many if-else statements. A dict is roughly as concise as a switch, and it's also much easier to work with and extend. Following the principle of "There should be one--and preferably only one--obvious way to do it," switches in Python are redundant. If you want branching logic, then use branching statements (if-else). If you want mapping, use a dict. The hiccup is that many programmers new to Python don't actually know what a dictionary is--or at least haven't used one before--so it's far from "obvious" to them to use it.
  • gknoy 10y ago
    Switch statements have fall-through behavior that you need `break` to avoid (in most languages); sometimes this is what you want, but only rarely. One trick I've seen people do in Python is to use a dict; define lambdas for each case, and then key the dict on your value that you would switch on, and invoke the lambda (or use existing functions). def fake_switch(self, thing): { 'foo': self.do_foo, 'bar': self.do_bar, 'frog': self.blast_vent_core }[ thing ]() I am doing a poor job of explaining _why_ one might want to do that, but one advantage is that you can verify that the right callbacks were called in tests, and also verify that each callback behaves right when invoked, since they are just references to methods or functions. I'm not sure if doing this would get marks for cleverness and testability, or would get one criticized for excessive cleverness. As the reviewer of the code when a coworker wrote it, I recall liking it, though.
• sssilver 10y ago
  > To the best of my ability to discern, the crux of the “good taste” requirement is the elimination of edge cases, which tend to reveal themselves as conditional statements. The fewer conditions you test for, the better your code “tastes”. looks at his Swift code and gulps
• Animats 10y ago
  Both versions suck. If "entry" is not found in the list, the code will run off the end of the list, either de-referencing zero and faulting or going off into junk, depending on how the list ends. Writing low-level list manipulation more than once sucks. It leads to bugs. Such manipulation should be encapsulated. That's why we have containers in modern languages. The Linux kernel is still C, not C++, which leads to too much of this sort of thing.
  • clarry 10y ago
    I think practicing C programmers are using the list implementations that were given to us two decades ago. Or any of the newer alternatives. And if they aren't, it's not because of the language.
• snovv_crash 10y ago
  One anti-pattern I've seen a few times, across multiple codebase and companies, is the for i in 1...cases: switch(i) To me, his first example fits this perfectly, except he is using ifs instead of a switch. Basically, if you are generating a big, trivial sequence and then filtering it down afterwards, think harder and probably there is a way to generate the sequence you are actually after directly.
  • Dylan16807 10y ago
    > One anti-pattern I've seen a few times, across multiple codebase and companies, is the for i in 1...cases: switch(i) Also known as the "for-case" loop. Version 2 is a prime example of this, and what not to do. It processes case 1, then case 2, then case 3, then case 4. The author rewrites it as one loop of 64, but four loops of 64 each would also be acceptable. > To me, his first example fits this perfectly, except he is using ifs instead of a switch. Basically, if you are generating a big, trivial sequence and then filtering it down afterwards, think harder and probably there is a way to generate the sequence you are actually after directly. Now you're talking about a different problem. The conditionals in the first version do not iterate through like code doing "switch(i)". I agree that it's better to directly generate than to filter, but that's a totally separate anti-pattern.
• Smaug123 10y ago
  I've been told many times that it's better to eliminate edge cases. I still don't really believe it as a universal law. When explaining the algorithm "remove an element from a linked list" to someone else, I would say "starting at the head, go along the list until you find the element; then delete the element". When explaining the algorithm "delete an element from a linked list", I would say "if you're at the head, update the head to be the next element; otherwise update the `next` property of the previous element to be the `next` property of the current element". It's so naturally a two-case problem that I'd be really surprised if anyone came up with the one-case answer first. Therefore, the two-case answer is the more readable to someone who has not seen the code before, because it corresponds to their intuition about how the algorithm should work. As a bonus, it's clear to me that the naive algorithm is correct, but I have to do mental work to convince myself that the one-liner algorithm is correct. (I'm saying nothing about the code; only the algorithms.) I agree much more with the reduced complexity of the later example (initialising the edges of a square), and I actually think it's worth double-counting the corners in this instance because the code is so much cleaner. (EDIT: Although I suppose the easy fix doesn't detract from the ease-of-reading, so actually it's avoidable.) In that instance, there's doubt about which is the natural algorithm to pick: one could conceivably come up with "initialise the top and bottom, then initialise the left and right", or "initialise the zeroth element of each side, then the first of each side, then…" as one's first attempt at solving the problem. Therefore I like this particular simplification.
  • marcosdumay 10y ago
    > I still don't really believe it as a universal law. If you were doing kernel development, you would. If you were writing enterprise CRUD applications, you wouldn't even think about it, and if you were developing hight throughput middleware, you'd switch that law on and off depending on where you are on the code. Or, at least, s/would/should/.
  • apk17 10y ago
    > It's so naturally a two-case problem that I'd be really surprised if anyone came up with the one-case answer first. [Raises hand] I always did it Linus' way in C (way before him); I don't remember if I ever had the need in assembly before.
• stirner 10y ago
  The author doesn't mention that the grid being initalized is square, and simply describes it as grid[rows][cols]. However, all his solutions are only correct for a square grid. One solution for arbitrary dimensions could be for (i = 0; i < rows; ++i) { grid[i][0] = 0; grid[i][cols - 1] = 0; } for (i = 0; i < cols; ++i) { grid[0][i] = 0; grid[rows - 1][i] = 0; }
  • deleted 10y ago
    [deleted]
  • dghf 10y ago
    You're updating the corners twice. Not really a problem with a simple assignment, but as no_protocol pointed out elsewhere [0], it could be if you ever wanted to change that to, say, an increment. Replace your second for statement with for (i = 1; i < cols - 1; ++i) { ... and you're golden. [0] https://news.ycombinator.com/item?id=12794235 https://news.ycombinator.com/item?id=12794235
    • stirner 10y ago
      You're right, my version is unoptimized because I was trying to keep the code neat as in the article. I think that optimizing the code for hypothetical future reuse is a bit premature. You could make a performance argument, but this optimization is going to prevent just 2 memory accesses, which aren't going to matter much compared to all the rest. If I was chasing performance I would start by memset'ing the top and bottom edges since they're contiguous in memory.
      • stirner 10y ago
        My mistake - the optimization would prevent four memory accesses. Still O(1).
• rimantas 10y ago
  Sandi Metz has a talk where she speaks about her dislike for if https://www.youtube.com/watch?v=OMPfEXIlTVE https://www.youtube.com/watch?v=OMPfEXIlTVE
  • majewsky 10y ago
    Thanks for the link. I found the talk very insightful. I would imagine Linus to disagree with Sandi's approach, though. She is never eliminating the if (as in: the conditional jump), just moving it from plain sight into the magic of dynamic method dispatch.
• jprzybyl 10y ago
  I'm reminded of a quote from Moore in "Thinking Forth": "A lot of conditionals arise from fuzzy thinking about the problem. In servo-control theory, a lot of people think that the algorithm for the servo ought to be different when the distance is great than when it is close. Far away, you’re in slew mode; closer to the target you’re in decelerate mode; very close you’re in hunt mode. You have to test how far you are to know which algorithm to apply." "I’ve worked out a non-linear servo-control algorithm that will handle full range. This approach eliminates the glitches at the transitioning points between one mode and the other. It eliminates the logic necessary to decide which algorithm to use. It eliminates your having to empirically determine the transition points. And of course, you have a much simpler program with one algorithm instead of three." "Instead of trying to get rid of conditionals, you’re best to question the underlying theory that led to the conditionals." That's part of a chapter of the book called Minimizing Control Structures. Forth guys are crazy about taste, and if I've learned anything from reading their stuff, it's that chasing tasteful programming to its end gets very hard. OP is right on the money. The hard thing is that it is a creative process, and takes a real understanding of the problem you're solving to do it. Worst of all, aside from the feeling of solving a puzzle well, the benefits only begin appearing much later. I'm glad the kernel team takes it seriously.
  • lallysingh 10y ago
    Link to the book: http://thinking-forth.sourceforge.net/ http://thinking-forth.sourceforge.net/
  • astrobe_ 10y ago
    Be it kernel programming or Forth, the cultural shock can hit hard. Most programming languages try their best to make things easier than easy, in order to maximize programmers' productivity. The focus on this parameter alone over-promotes the "worse is better" or "good enough" mindset. That doesn't fare well with kernel programming. It's even worse with Forth. As soon as your "word" (function in Forth lingo) has to deal with more than two or three variables, you're in trouble. As soon as you have more than two levels of control-flow structures, nightmares begin. If you're looking for programming practice then I recommend Forth. It may or may not be a "practical" language for you, but it makes you really think about complexity.
• deanCommie 10y ago
  I don't know how to reconcile Linus's track record is of indisputable brilliance and success (I use Linux and Git on a daily basis and am eternally grateful for both), with the fact that I would absolutely DESPISE working with a peer with the kinds of attitudes and opinions on coding that Linus has. I think the problem I have is that a lot of people use Linus's examples and stories as justification for their own suboptimal interpersonal skills, overly-clever code that only they understand, but without the same level of brilliance or track record to justify it. Ultimately I think Linus's contributions to Software are pantheon, but he should not be looked to for imitation or lessons. His lessons for success are detrimental to the vast majority of software engineers.
  • johan_larson 10y ago
    Has Linus written at length about how to write code well? Or do his notions of it have to be inferred from his actions and shorter commentary?
  • Noseshine 10y ago
    You make claim after claim, and I think they do need substantiating evidence. After all, I find it highly unlikely that if he were even 10% as bad as you make it sound he would be the head of one of the most successful projects - especially since everybody working with him does so voluntarily. There would have been breaks long ago. The fact that the Linux kernel held together under the original author loudly speaks against assumptions of "bad interpersonal skills" on the side of Linus.
    • YCode 10y ago
      Some food for thought: http://www.computerworld.com/article/3004387/it-management/how-bad-a-boss-is-linus-torvalds.html http://www.computerworld.com/article/3004387/it-management/h... "I’m not a nice person, and I don’t care about you. I care about the technology and the kernel — that’s what’s important to me."
      • Noseshine 10y ago
• licorna 10y ago
  You can think of your grid array as a contiguous piece of memory. I would do: bzero(grid, sizeof(int) * COLS); bzero(grid + COLS * (ROWS - 1), sizeof(int) * COLS); for (int i=1; i < ROWS-1; i++) { *(grid + (i * COLS)) = 0; /* set left col to 0 */ *(grid + (i * COLS) + COLS - 1) = 0; /* set right col to 0 */ }
• sytelus 10y ago
  Here's even more "tastier" 2 lines version: remove_list_entry(entry) { for (indirect = &head; (*indirect) != entry; indirect = &(*indirect)->next); *indirect = entry->next; } The big problem: both Linus's and above versions don't handle the case if entry wasn't found, for example, if entry was null. This is the fundamental issue with trying make code overly compact: sometime you lose the sight of important edge cases. Segfaults are not tasty. I generally prefer to write code that reflects my thought process and avoid unnecessarily try making it compact. As Knuth had said "Programs are meant to be read by humans and only incidentally for computers to execute”. Uglier versions allows to explicitly documents edge cases. This enables future maintainer to make sure these cases are covered when s/he makes code changes. Obviously taking this to another extreme would ruin this. The better taste lies somewhere between the compact Linus's version and some zeolite's too verbose version. Small problem: Another thing to think about is extra pointer redirection required in Linus's code. This is fine for most cases but if I was dealing with very large list over and over then that's unnecessary perf hit.
  • thomasahle 10y ago
    The 'entry wasn't found' case is an entirely different issue than what Linus is showcasing. It may or may not be relevant for the particular function, depending on its contract and documentation.
• k__ 10y ago
  Well, the "good taste" example has a lower cyclomatic complexity, so it is at least a measurable change.
• troydj 10y ago
  Linus' "good" version has a McCabe cyclomatic complexity of 2, whereas the "bad" version has a value of 3. So, objectively, one could argue there is improvement there (albeit small). Validation of the "good" version will be easier (e.g. code coverage testing) with fewer paths through the code. Additionally, a lower cyclomatic complexity typically implies less stress on the developer's working memory while reading code (since you don't have to consume an internal brain "register" holding the result of a conditional while following the flow of control).
  • k__ 10y ago
    I thought the same. On the other hand it took me a bit of time to figure out what's going on. But I'm not C dev, so that could be the issue here.
    • dwc 10y ago
      Serious question: after figuring it out, did you have a somewhat better grasp of linked list mechanics than before?
      • rocqua 10y ago
        I had a better grasp of how to work with them (am not who you replied too). I never had to build my own linked list though. I never took those basic courses.
• e19293001 10y ago
  Maybe I'm wrong about this but I noticed that the code didn't call free() to the element that has breen removed. That would cause a memory leak. Am I missing something here?
  • halomru 10y ago
    The caller still holds a pointer to the element and can free it. I would argue that delegating the responsibility of freeing the element to the caller is more performant, since that allows the code to reuse that list element and insert it into a different (compatible) list. That's the kind of performance/ease-of-use trade-off that sounds reasonable in an open source project with strict code reviews (but in very few other places)
• deleted 10y ago
  [deleted]
• kutkloon7 10y ago
  I think the main lesson that's there, is that in edge cases, it is sometimes helpful to work with pointers which point to the structures you're actually working with, instead of considering the structures themselves. This especially seems to work for getting rid of null checks. This takes some getting used to, but it allows you to write more efficient code. I would do this in kernel development (smart people, high demands on performance), but I would not recommend to write code like this is enterprise software. When you zoom out a bit, and describe the task with natural language, you can also see that this might be a good idea: "Find the node that is equal to entry, then, if the previous node exists, set the 'next' pointer of the previous node to the 'next' pointer of the current node." sounds a lot more tedious than "Find the pointer that points to entry, and set it to the 'next' pointer of entry.". This also works for more complicated tasks: // removes all instances from the linked list and return the number of entries removed int removeentries(valuetoremove, list) { removedentries = 0; // how many entries we've removed prevpt = NULL; // pointer to previous entrypt = list.head; // get pointer to first node // walk the list (be careful, head can be a nullpointer) while (entrypt) { if (*entrypt == valuetoremove) if (prevpt) { prevpt->next = entrypt->next; removedentries++; } prevpt = entrypt; // save current node as prev for next node entrypt = entrypt->next; // move to next node } return removedentries; } can be re-written to: int removeentries(valuetoremove) { removedentries = 0; // how many entries we've removed indirect = &head; // points to the current node // walk the list while (indirect) { // remove current entry from list if its value equals valuetoremove if (indirect->value == valuetoremove) { *indirect = *indirect->nextpt; removedentries++; } indirect = indirect->nextpt; } return removedentries; }
• usefulcat 10y ago
  Is it just me or does the grid initialization example seem extremely contrived?
  • vilya 10y ago
    I thought that too. I found it hard to believe that anyone would write either of the first two versions without having thought of the 3rd - or, you know, just having 2 or 4 separate for loops. Maybe there was something about the problem's context that the author left out of the post though?
• facundo_olano 10y ago
  All due respect to Linus, but the second example is much less readable. Proof of that: the code was reduced in half, but the amount of comments was doubled. I think concepts like "elegance" and "good taste" of code should exist, but they are obviously subjective. My choice of good taste always considers readability first.
  • dpark 10y ago
    I'm pretty sure that the comments are for people who Linus doesn't want contributing to the kernel. This code would likely have no comments at all if checked into the Linux kernel because it would be obvious to everyone allowed to do commits.
• jordigh 10y ago
  The example there about edges on an array is something I've had to directly deal with myself when I implemented a multidimensional image-processing function for GNU Octave. The problem is to find connected components in a binary image, where voxels are either 0 or 1. You want to find all the islands of ones, and you want to be flexible if diagonals count as being connected or not. The problem, of course, is that along the edges you don't want to check for neighbours outside of the image boundary. I found no good way to do this for an n-dimensional image, after a few attempts of writing very complicated code. In the end, I ended up padding the whole image with a boundary of zeros, iterating over the padded image with an "if(current_voxel)" conditional that skipped checking for lit neighbours around the boundary, and when checking for lit neighbours at the original image's boundaries would give no neighbours at the padded zero boundaries. The code was cleaner, but I incurred a big realloc, because N-dimensional images in Octave are stored as contiguous Fortran-order (column-major) arrays. I'm still looking for a better solution to this problem. So, how do you do this cleanly?
  • arghbleargh 10y ago
    Isn't it simple enough to make three bounds checks to essentially do the same thing you did with the zero padding? You can even make it a function is_in_bounds(x, y, z)
    • jordigh 10y ago
      It's n-dimensional, not 3-dimensional. Also, it's really slow and wasteful to be doing bounds-checking for each voxel of an image that is almost completely not a boundary, whereas you can't avoid checking if each voxel is lit. :-)
  • ThatGeoGuy 10y ago
    I think there are three generally accepted ways to do this: 1) Pad with zeros until you fill your extent 2) Pad with an arbitrary constant of some kind 3) Pretend that spilling over one edge loops back to the opposite edge, as if the image was just a view over a repeating signal in that dimension. The first two are naive and work well enough for simple tasks. The third actually has some basis to it, although it does seem odd. The third option is actually how the image is treated when you transform the image using a 2D Fourier transform. Instead of cutting off the "signal" and padding with zeros, it assumes that the signal is continuous and repeating (i.e. it is a 2D or N-dimensional Fourier series). I know MATLAB uses this as the default for some (irritatingly not _all_) of it's operations, especially those concerned with Fourier transforms. I can't speak to Octave, but I believe they try to aim for bug compatibility with MATLAB where possible. One nice property of this kind of behaviour is that it avoids introducing some artefacts in the image when performing some sort of kernel filter or convolution. Note that the interesting bit here is that Python + Numpy actually makes this whole debacle a bit easier to deal with. You can implement solution 3) easier than 1) or 2) with Python, since you can use negative indices in any iterable. So for an image `I`, you can call `I[xdim, ydim, -1, wdim]` for any N-dimensional image and the wrapping works well. Of course you then have to ensure you don't go past the "extent" of your N-dimensional space, but that's simple enough to enforce with 0 and `I.shape`. A lot of people complain that negative indices make no sense and are an annoying language flaw, but this is one scenario where I'm really glad they're available. Numpy views would also probably alleviate your realloc problem, since you can use views to simulate extents within larger matrices, without requiring copying / separate allocation.
• nurettin 10y ago
  Often I find myself solving problems in a single performant SQL with lateral/cross joins before hitting the business logic code in order to simplify the logic part into a single loop with no conditionals/edge cases. Turns out this is not the best practice because you will have to explain all about your clever use of olap functions and lateral joins to someone less knowledgeable who has to append to your code.
  • dragonwriter 10y ago
    I would argue that having developers that don't understand and won't bother to learn SQL working on application dependent on SQL-based DBs is the not-best-practice there, not proper and efficient use of SQL by developers who do know what they are doing.
    • nurettin 10y ago
      You are right, of course, but they have "working knowledge" of sql which is adequate for most reports and operations. Just not the "high level" SQL someone armed with the knowledge of functional programming could conjure when faced with the alternative of writing lots of nested conditional statements and loops in an imperative language because it would bore him to pieces.
• deleted 10y ago
  [deleted]
• saosebastiao 10y ago
  I know Sufficiently Smart Compilers are a running joke, and I know a lot of people think the idea is a bad one, but come on...this should be the job of a compiler. The first example is way more understandable. Yes, it branches, and yes it is more code and less efficient. But a compiler should be able to derive the more efficient version from the more understandable code. And it's a shame that it doesn't.
  • zodiac 10y ago
    I think Linus argued that the second version is more understandable. In fact if you read the article there's no claim being made that the second version is more efficient, and I don't think it is
• leothekim 10y ago
  This article made me think of this Gordon Bell quote: "The cheapest, fastest, and most reliable components are those that aren’t there."[1] That quote is a reminder to me that there is a certain artistry behind engineering anything. [1] http://quotes.cat-v.org/programming/ http://quotes.cat-v.org/programming/
• LordHumungous 10y ago
  I'm a bit confused. If the entry is not in the list, does Linus's while loop throw a seg fault? Or are we assuming that the list contains the entry?
• delinka 10y ago
  Does it follow that attempting to write code that is closer to functional style also improves the code's "tastefulness?" I'm under the impression that better functional code contains fewer branches.
• stevebmark 10y ago
  Eventually you will realize Linus is a bad person to take advice from and stop idolizing him.
  • sigjuice 10y ago
    Examples of good persons to idolize, please? :-)
    • stevebmark 10y ago
      I'm an anonymous internet poster, I don't have a suggestion for who you should idolize. But picking Linus is a red flag. His man-child entitled attitude, condescension, arrogance, etc, make him a parody of a toxic developer that ruins teams from the inside out. Normalizing and uplifting that behavior makes us all worse off.
• tbabb 10y ago
  Oh good lord. That edge-initialization code made my jaw drop. Did he not think about the problem _at all_ before putting code down?
• WesternStar 10y ago
  For the curious like me who wanted to see the assembly for the Linus examples and compare https://godbolt.org/g/sqdBrf https://godbolt.org/g/sqdBrf. Most importantly they have the exact same number of branches and their loops are exactly the same size 4 instructions on all opt levels greater than 02. The setup code for the first one is much longer.
  • WesternStar 10y ago
    Here is also a somewhat representative example of various grid initialization pay attention to version 3 where the compiler unrolls the loop in O3 its likely the fastest version. https://godbolt.org/g/mMBhuW https://godbolt.org/g/mMBhuW
• sde 10y ago
  The code in the examples is not valid in any historical version of C. The use of the -> operator with an implicit int as the left argument ("entry" in the examples) was only valid from 1975 in Unix C (but not in GCOS C) until K&R C in 1978. The use of C++ style comments ("//") is only valid in standard C since 1999. There's no point in arguing over pseudocode as if it's C.
• harry8 10y ago
  Both are correct, one is considered better based on a subjective assessment. People who disagree with that the version a person selects is objectively better have no taste.
• jaakl 10y ago
  Every time you write an 'if' and do not stop for a moment to consider it, a kitten dies.