Applying the Linus Torvalds “Good Taste” Coding Requirement (article)
-
Article examines a refactoring proposed by Linus Torvalds using a linked list example.
Before:
After:
Then it goes on to describe how OP refactored some of his own code using similar principles.
IMO, the OP's refactoring should have been obvious with or without Linus's "good taste" example. What I find more interesting is how I actually prefer the code in the first slide to the second.
The first is easy to follow, easy to understand. If you went into it using a debugger, you'd probably get nicely separated values and be able to follow along. The second slide is just too much dirty pointer magic and needless shorthand. I need to stop and think what's going on.
I can't tell if this is due to my inexperience with C, though. In languages I'm more familiar with, I sometimes use shorthand that might look equally mysterious to newbies (eg.
!!xin javascript).I guess it's all the function of how familiar are you with a certain coding environment.
-
@cartman82 said in Applying the Linus Torvalds “Good Taste” Coding Requirement (article):
The first is easy to follow, easy to understand. If you went into it using a debugger, you'd probably get nicely separated values and be able to follow along. The second slide is just too much dirty pointer magic and needless shorthand. I need to stop and think what's going on.
Agreed. The article talks about conditionals exposing edge cases, and trying to eliminate them by covering everything in one. That's fine in principle, but sometimes it's better to explicitly show the edge cases, to make it better for the maintainer who'll come next and might not understand your "clever" "tasteful" code.
@cartman82 said in Applying the Linus Torvalds “Good Taste” Coding Requirement (article):
IMO, the OP's refactoring should have been obvious with or without Linus's "good taste" example
Yeah, it's pretty inefficient and most loop iterations don't do anything.
-
@cartman82 said in Applying the Linus Torvalds “Good Taste” Coding Requirement (article):
The second slide is just too much dirty pointer magic and needless shorthand.
The variable declarations would help here. If I had seen
indirectbeing declared as a pointer to a pointer, I would have understood the code immediately.@cartman82 said in Applying the Linus Torvalds “Good Taste” Coding Requirement (article):
I can't tell if this is due to my inexperience with C, though.
Yup. If you think this is dirty pointer magic, you've never seen truly dirty pointer magic. :P
-
I try to think and code in what Linus describes as good taste. Sometimes I fail because who gives a shit? This code is easy to understand and works and I don't have time to mess around with it. Sometimes I fail because there really are too many special snowflake edge conditions to simplify the algorithm.
Often good taste code avoids potential problems, like a bad comparison in the grid initialization example.
I'm with @cartman82 on the linked list example, though. I don't think the pointer indirection is worth getting rid of the conditional. I also think it was extremely bad taste to not use braces for
ifs andwhiles.
-
@boomzilla said in Applying the Linus Torvalds “Good Taste” Coding Requirement (article):
I also think it was extremely bad taste to not use braces for ifs and whiles.
Fuck useless braces that waste vertical space
-
@fbmac said in Applying the Linus Torvalds “Good Taste” Coding Requirement (article):
waste vertical space
Yeah, I hear there's a big shortage of lines. It's going to be the next IPV4
-
This post is deleted!
-
@fbmac just wasted two lines of text with that deletion. Fuck him
-
For me, it's much more important to have code that is easy to read and understand than the simplest possible algorithm. Yes, of course, when it becomes a performance issue you have to do something, but on the whole, it's much easier to misunderstand "optimal" code and either miss a bug or introduce one while tweaking something.
The first slide is for me much easier to follow than the second one. It's clear what it does, I can go through it without needing to picture pointers-to-pointers. Everything that adds mental overhead while reading the code is bad for me as it forces me to focus on the mechanics rather than the idea.
That being said, I probably wrote less than 0.0...01% of the code that Linus has written, so for him that might seem obvious. Basically, I'd say, write code that you are comfortable with. You can never know what future maintainers (including yourself!) will consider "obvious", so your only comparison point is you and now.
-
@cartman82 said in Applying the Linus Torvalds “Good Taste” Coding Requirement (article):
IMO, the OP's refactoring should have been obvious with or without Linus's "good taste" example.
Eh, I don't know. I'd probably write it like in the first example and only reach for the method in the second if there were serious performance issues with that code.
The problem is, Linus is a kernel maintainer. And sure, in an OS kernel or a standard library, it's pretty crucial that your linked list walking code is as efficient as possible. All those superfluous pointer assignments add up.
But in application code that's not on a critical performance path, it just says "hey, look at me, I'm clever!". And the 5 more seconds I need to spend tracing this code in my head are probably worth more than any sort of performance gains I could get from it.
Spending an hour to shave off a single instruction is something I do for fun in TIS-100 or Human Resource Machine, not something I'm comfortable billing a client for.
-
In neither version does he check if the element to remove is in the list at all.
If he is so sure that the element is in the list, he probably has a reference to it anyway, so can remove it without iterating.Also in the first version the order is backwards: try to iterate the list and THEN check if the element was at the beginning?
The proposed refactoring is hard to generalize as it relies on C++'s pointer behaviour.
If we are using a linked-list and aiming for most clear code, we might as well use functional programming's recursive approach to linked lists.
-
@Maciejasjmj said in Applying the Linus Torvalds “Good Taste” Coding Requirement (article):
But in application code that's not on a critical performance path, it just says "hey, look at me, I'm clever!". And the 5 more seconds I need to spend tracing this code in my head are probably worth more than any sort of performance gains I could get from it.
This
-
@cartman82 said in Applying the Linus Torvalds “Good Taste” Coding Requirement (article):
@Maciejasjmj said in Applying the Linus Torvalds “Good Taste” Coding Requirement (article):
But in application code that's not on a critical performance path, it just says "hey, look at me, I'm clever!". And the 5 more seconds I need to spend tracing this code in my head are probably worth more than any sort of performance gains I could get from it.
This
Yes!
"Everyone knows that debugging is twice as hard as writing a program in the first place. So if you're as clever as you can be when you write it, how will you ever debug it?"
(from Kernighan, says the All-Mighty Web)
-
@Adynathos said in Applying the Linus Torvalds “Good Taste” Coding Requirement (article):
In neither version does he check if the element to remove is in the list at all.
If he is so sure that the element is in the list, he probably has a reference to it anyway, so can remove it without iterating.If all he has is a pointer (reference) to the element, then removing from single-linked list requires iteration and there's no way around it.
@Adynathos said in Applying the Linus Torvalds “Good Taste” Coding Requirement (article):
Also in the first version the order is backwards: try to iterate the list and THEN check if the element was at the beginning?
It doesn't matter much, considering the loop doesn't run at all if you're at head.
@Adynathos said in Applying the Linus Torvalds “Good Taste” Coding Requirement (article):
The proposed refactoring is hard to generalize as it relies on C++'s pointer behaviour.
C's pointer. Also, it doesn't rely on pointer implementation of any particular language, but rather the concept of a reference to reference - something that Java and C# suck for not having.
@Adynathos said in Applying the Linus Torvalds “Good Taste” Coding Requirement (article):
If we are using a linked-list and aiming for most clear code, we might as well use functional programming's recursive approach to linked lists.
And rely on tail call optimization for decent performance in the guts of kernel in a language that has no guarantee whatsoever of tail call optimization?
-
@cartman82 said in Applying the Linus Torvalds “Good Taste” Coding Requirement (article):
I actually prefer the code in the first slide to the second.
In C, a lot of the time verbosity is preferred over elegance because elegance is just plain difficult to read. C was not designed for elegance, and you really have to let it sink into your brain a while before you can read elegant code.
But this... I really like this article. It encapsulates a lot of the code smell stuff I've been running into inside SockDrawer. Things where, I know this condition is an edge case the way I've written it now, but there's gotta be an algorithm that seamlessly blends the edge cases into main cases. I'm not very good yet, but I'm finding the answer more and more often.
It's the difference between "Meh, it works" and doing a good job building something, in my opinion.
-
@Maciejasjmj said in Applying the Linus Torvalds “Good Taste” Coding Requirement (article):
The problem is, Linus is a kernel maintainer. And sure, in an OS kernel or a standard library, it's pretty crucial that your linked list walking code is as efficient as possible. All those superfluous pointer assignments add up.
Yes, but I wouldn't assume that the smart-ass version is more efficient without going and actually looking at the machine code that the compiler produces, both in terms of number of instructions and number of CPU cycles across relevant architectures. The simpler code is much easier to optimize mechanically…
-
@dkf said in Applying the Linus Torvalds “Good Taste” Coding Requirement (article):
I wouldn't assume that the smart-ass version is more efficient without going and actually looking at the machine code that the compiler produces, both in terms of number of instructions and number of CPU cycles across relevant architectures.
This.
It used to be easy to outperform compiler-generated code. Much harder in the 21st century.
-
@Gąska said in Applying the Linus Torvalds “Good Taste” Coding Requirement (article):
It doesn't matter much, considering the loop doesn't run at all if you're at head.
But the point of the article is clarity of the code. The loop is only needed in one of the if's branches, so why not put it there.
it doesn't rely on pointer implementation of any particular language, but rather the concept of a reference to reference
This concept of references to variables is not present in many widely used languages.
In the context of clarity, it makes the variable non-local - someone somewhere may have a reference to the variable and change it and it may be hard to remember where the references are kept.
Of course the same non-locality applies to objects - but with the idea of encapsulation we assume the object is managing its members they are not to be trusted in the same way local variables are.And rely on tail call optimization for decent performance in the guts of kernel in a language that has no guarantee whatsoever of tail call optimization?
Readability and performance are orthogonal. Readability can be sacrificed to get max performance - by using very low level programming like in the kernel.
-
@Yamikuronue said in Applying the Linus Torvalds “Good Taste” Coding Requirement (article):
But this... I really like this article. It encapsulates a lot of the code smell stuff I've been running into inside SockDrawer. Things where, I know this condition is an edge case the way I've written it now, but there's gotta be an algorithm that seamlessly blends the edge cases into main cases. I'm not very good yet, but I'm finding the answer more and more often.
It's the difference between "Meh, it works" and doing a good job building something, in my opinion.
I think it can demonstrate a deeper understanding of the task. Assuming you get it right, of course. It's like setting up any process (e.g., interface, workflow, manufacturing) where you eliminate certain errors just by the way you set it up.
The trap is the
s that show up later. A less elegant but straightforward approach is sometimes easier to tailor. But then, sometimes it's the opposite.
-
@boomzilla said in Applying the Linus Torvalds “Good Taste” Coding Requirement (article):
A less elegant but straightforward approach is sometimes easier to tailor. But then, sometimes it's the opposite.
I usually see the latter, but I don't usually begin with a very elegant situation to begin with :) I tend to come back to a method, find it a tangled mess of if-statements, and cringe at the idea of adding one more.
-
Like this crap:
then((post) => { let actionToken = 'target'; /* Group types*/ if (actor.hasProperty('scum') || actor.hasProperty('mafia')) { //Revoke previous scum action const prevAction = game.getActionOfType('target', null, 'scum'); if (prevAction) { prevAction.revoke(post.id); } actionToken = 'scum'; } if (actor.hasProperty('scum2')) { //Revoke previous scum action const prevAction = game.getActionOfType('target', null, 'scum2'); if (prevAction) { prevAction.revoke(post.id); } actionToken = 'scum2'; } if (actor.hasProperty('cultLeader')) { actionToken = 'cult'; } return game.registerAction(post.id, actor.username, target.username, 'target', actionToken); })
-
@Yamikuronue That code makes me wince a bit but I'll say that it's mitigated a little by the fact that there are tests (if I recall) when those additional cases and such are added (which also helps you verify correct behavior when you refactor).
-
@Adynathos said in Applying the Linus Torvalds “Good Taste” Coding Requirement (article):
This concept of references to variables is not present in many widely used languages.
In the context of clarity, it makes the variable non-local - someone somewhere may have a reference to the variable and change it and it may be hard to remember where the references are kept.I'm not sure how much to mock you for conflating the concepts of variables, values, objects, references and memory, so I won't.
@Adynathos said in Applying the Linus Torvalds “Good Taste” Coding Requirement (article):
Of course the same non-locality applies to objects - but with the idea of encapsulation we assume the object is managing its members they are not to be trusted in the same way local variables are.
I don't quite get why you worry about how much trust we put in things we don't know about (because the whole point of encapsulation is to make external environment not know about internals). I'm sure you have a point of some sort, but I honestly cannot decipher it.
@Adynathos said in Applying the Linus Torvalds “Good Taste” Coding Requirement (article):
Readability and performance are orthogonal. Readability can be sacrificed to get max performance - by using very low level programming like in the kernel.
It doesn't sound so orthogonal when there's a trade-off to make. Also, replacing recursion with iteration isn't that low level.
-
@Maciejasjmj said in Applying the Linus Torvalds “Good Taste” Coding Requirement (article):
Spending an hour to shave off a single instruction is something I do for fun in TIS-100 or Human Resource Machine, not something I'm comfortable billing a client for.
In that case, you're obviously looking for which edge cases the game will allow you to break. I cringed at shenzhen IO like that last week - it had you compare two 10-digit numbers, and I figured I'd try a hash function. The embarrassingly-simple sum-of-digits passed all tests.
Regarding the original item, I'm in the double-indirection camp. I write that code often enough for it to be a common pattern, and having 2 variables to keep track of instead of one also adds complexity.
Also the double-indirect code, without the extravagant commenting, is more compact, and IIRC Linux swears by an 80x25 terminal window?
-
@heterodox said in Applying the Linus Torvalds “Good Taste” Coding Requirement (article):
there are tests (if I recall)
Oh yes, loads of them. I hate that method, this is just the worst of the promise chains in it. It's clunky as fuck.
-
@cartman82 said in Applying the Linus Torvalds “Good Taste” Coding Requirement (article):
(eg.
!!xin javascript)My favorite new trick is
if (~foo.indexOf(bar)) "bar is in foo"; else "bar is not in foo";
-
@Yamikuronue said in Applying the Linus Torvalds “Good Taste” Coding Requirement (article):
It's clunky as fuck.
It's got all sorts of weird business rules in it, such as mapping
mafiaandscumto each other…
-
@fbmac said in Applying the Linus Torvalds “Good Taste” Coding Requirement (article):
Fuck useless braces that waste vertical space
Apple agrees with you and created a bug in their SSL library to show how awful it is
-
@dkf said in Applying the Linus Torvalds “Good Taste” Coding Requirement (article):
such as mapping mafia and scum to each other…
That I could drop without issue, since nobody's using it yet. The bigger issue is offering two scum groups, flagging cults, and having the groups act as if they had one action instead of many
-
@Yamikuronue At first glance it looks like it might be easy to refactor. But it turns out there's just enough different things in different cases that you're probably still better off with what you have now.
-
@flabdablet said in Applying the Linus Torvalds “Good Taste” Coding Requirement (article):
@dkf said in Applying the Linus Torvalds “Good Taste” Coding Requirement (article):
I wouldn't assume that the smart-ass version is more efficient without going and actually looking at the machine code that the compiler produces, both in terms of number of instructions and number of CPU cycles across relevant architectures.
This.
It used to be easy to outperform compiler-generated code. Much harder in the 21st century.
True, but I'd still be surprised if there was a compiler that knows how to optimize away that conditional. Every conditional is a pipeline stall waiting to happen; that alone can easily double the execution time of the entire function on a modern CPU. The conditional Linus eliminated is particularly prone to misprediction if the CPU only has a static predictor or the branch cache is still cold.
-
@cartman82 Yeah, right? Like, registerAction could revoke the previous action, but it doesn't know which kinds of tokens care about the user vs which ones don't. It could do it unless the token is undefined, I guess? And I could have a map of properties to tokens? Or I could condense all three into one use case based on if they have any property in my map? But it has to test them one by one because that's the interface I have exposed. It's weird.
-
@LaoC said in Applying the Linus Torvalds “Good Taste” Coding Requirement (article):
I'd still be surprised if there was a compiler that knows how to optimize away that conditional.
Be surprised.
test.c:static struct node { struct node *next; } *head; void remove_list_entry(struct node *entry) { struct node *prev = 0; struct node *walk = head; // Walk the list while (walk != entry) { prev = walk; walk = walk->next; } // Remove the entry by updating the // head or the previous entry if (!prev) { head = entry->next; } else { prev->next = entry->next; } } void remove_list_entry_2(struct node *entry) { // The "indirect" pointer points to the // *address* of the thing we'll update struct node **indirect = &head; // Walk the list, looking for the thing that // points to the entry we want to remove while ((*indirect) != entry) { indirect = &(*indirect)->next; } // .. and just remove it *indirect = entry->next; }gcc -O3 -S test.cgeneratestest.s:[snip] remove_list_entry: .LFB0: .cfi_startproc movq head(%rip), %rdx cmpq %rdi, %rdx jne .L3 jmp .L8 .p2align 4,,10 .p2align 3 .L5: movq %rax, %rdx .L3: movq (%rdx), %rax cmpq %rax, %rdi jne .L5 movq (%rdi), %rax movq %rax, (%rdx) ret .L8: movq (%rdx), %rax movq %rax, head(%rip) ret .cfi_endproc [snip] remove_list_entry_2: .LFB1: .cfi_startproc movq head(%rip), %rdx cmpq %rdx, %rdi jne .L11 jmp .L15 .p2align 4,,10 .p2align 3 .L13: movq %rax, %rdx .L11: movq (%rdx), %rax cmpq %rax, %rdi jne .L13 .L10: movq (%rdi), %rax movq %rax, (%rdx) ret .L15: movl $head, %edx jmp .L10 .cfi_endprocThe compiler has worked out that the only time it needs to write back to
headis if the while loop never runs even once, so the conditional in question gets hoisted to the start of the proc amongst the jumps that set up the loop. The "optimized" version actually ends up costing an extra jump in order to force updating the header to use the same code as updating a walked-to link.
-
@flabdablet said in Applying the Linus Torvalds “Good Taste” Coding Requirement (article):
The compiler has worked out that the only time it needs to write back to
headis if the while loop never runs even once, so the conditional in question gets hoisted to the start of the proc amongst the jumps that set up the loop. The "optimized" version actually ends up costing an extra jump in order to force updating the header to use the same code as updating a walked-to link.Whow. Nifty indeed! I'd like to retract everything and argue for the opposite now (I hear that's en vogue here now) :)
Butbutbut: the extra jump is probably free anyway.
-
@LaoC said in Applying the Linus Torvalds “Good Taste” Coding Requirement (article):
the extra jump is probably free anyway.
Maybe.
I'd expect the following to run marginally faster:
void remove_list_entry_3(struct node *entry) { struct node *ping = head; struct node *pong = 0; // Handle header as special case if (head == entry) { head = head->next; return; } // Walk the list do { pong = ping->next; if (pong == entry) { ping->next = pong->next; return; } ping = pong->next; } while (ping != entry); pong->next = ping->next; }It compiles to this:
remove_list_entry_3: .LFB2: .cfi_startproc movq head(%rip), %rax cmpq %rdi, %rax jne .L21 jmp .L23 .p2align 4,,10 .p2align 3 .L19: movq (%rdx), %rax cmpq %rax, %rdi je .L24 .L21: movq (%rax), %rdx cmpq %rdx, %rdi jne .L19 movq (%rdi), %rdx movq %rdx, (%rax) ret .p2align 4,,10 .p2align 3 .L24: movq (%rdi), %rax movq %rax, (%rdx) ret .L23: movq (%rax), %rax movq %rax, head(%rip) ret .cfi_endprocThis eliminates a redundant register move from the inner loop, which will still easily fit inside a single cache line. I would expect the branch predictor to get the forward
jeand backwardjneright most of the time.
-
@boomzilla said in Applying the Linus Torvalds “Good Taste” Coding Requirement (article):
Sometimes I fail because who gives a shit? This code is easy to understand and works and I don't have time to mess around with it.
^ I agree with what @blakeyrat says.
-
@flabdablet said in Applying the Linus Torvalds “Good Taste” Coding Requirement (article):
The compiler has worked out that the only time it needs to write back to head is if the while loop never runs even once, so the conditional in question gets hoisted to the start of the proc amongst the jumps that set up the loop. The "optimized" version actually ends up costing an extra jump in order to force updating the header to use the same code as updating a walked-to link.
Basically, modern compilers transform code pretty thoroughly on the way from what you write to what they issue. Some of the main changes they do are to turn variables into simpler forms that are only assigned to once, and to try to get rid of explicit writes into the stack frame (so the stack only really gets used where a variable has to be spilled). These allow some pretty deep reconstruction of code.
The general rule is write clear code (because you're going to be stuck reading it) that uses the right algorithm, and only start fiddling around with optimising things if really necessary. And always test whether your “optimisations” actually benefit things; if they don't speed things up enough, don't put them in because the loss of readability isn't justifiable.
-
@dkf but the indirect pointer version is pretty clear, and I assume compilers weren't that smart when Linus created the habit of writing code like that
-
@fbmac said in Applying the Linus Torvalds “Good Taste” Coding Requirement (article):
but the indirect pointer version is pretty clear
It gives me the heebie-jeebies. :)
I tend to assume that if I write very plain code, the compiler will eventually become smart enough to do good things with that code. I don't assume that when it comes to smartass code, and I would be very hesitant about what the compiler generates without actually looking at it and measuring the performance.
Of course, I'm also not that fond of linked lists in the first place. There are good reasons to use them sometimes, but they've a tendency to have poor memory locality.
-
@boomzilla said in Applying the Linus Torvalds “Good Taste” Coding Requirement (article):
I try to think and code in what Linus describes as good taste. Sometimes I fail because who gives a shit? This code is easy to understand and works and I don't have time to mess around with it. Sometimes I fail because there really are too many special snowflake edge conditions to simplify the algorithm.
Often good taste code avoids potential problems, like a bad comparison in the grid initialization example.
I'm with @cartman82 on the linked list example, though. I don't think the pointer indirection is worth getting rid of the conditional. I also think it was extremely bad taste to not use braces for
ifs andwhiles.I accept short ifs without braces, when the statement is short and when there's only an if. When there is an else, fuck you, put those braces around and add the required new lines.
Hate stuff like this
if (stuff) doStuff(); else makeStuff();
-
@kt_ said in Applying the Linus Torvalds “Good Taste” Coding Requirement (article):
I accept short ifs without braces, when the statement is short and when there's only an if.
goto fail:
-
@kt_ I'm glad I work in a team where nobody complains about the presence or absence of braces, spaces vs tabs, or any other "coding style" bullshit.
-
@fbmac said in Applying the Linus Torvalds “Good Taste” Coding Requirement (article):
"coding style" bullshit.
Be careful, literal wars have been started for lesser insults.
-
@accalia figuratively speaking, of course.
-
@Yamikuronuehighlight.js said in Applying the Linus Torvalds “Good Taste” Coding Requirement (article):Like this crap:
<div class="content" component="post/content" itemprop="text"> <p>Like this crap:</p> <pre class="markdown-highlight"><code class="hljs php"> ... </code></pre> </div>Yup, looks like PHP to me. I mean, just look at all the dollar signs in that code!
Anyway...
While I don't play any of the forum's games and I lack the knowledge about how game, actors, and actions of different types work and relate to each other, here's a shot:
const TOKEN_REVOKATIONS = [ //Lower priority first { token: 'scum' , props: ['scum', 'mafia'], }, // aligned for { token: 'scum2', props: ['scum2' ], }, // your viewing { token: 'cult' , props: ['cultLeader' ], }, // pleasure ] function actorHasOneOfProperties( actor, properties ){ for( let prop of properties ){ if( actor.hasProperty(prop){ return true; } } return false; } // ... then( (post) => { // Functional programming ftw let actionToken = TOKEN_REVOKATIONS.reduce( (prevToken, currAction) => { if( actorHasOneOfProperties(actor, currAction.props) ){ let prevAction = game.getActionOfType('target', null, currAction.token); // If I understand this correctly if( prevAction ){ prevAction.revoke(post.id); } return currAction.token; } else { return prevToken; } }, 'target'); return game.registerAction(post.id, actor.username, target.username, 'target', actionToken); })Is that better than your code? Worse? You decide. I don't know how the revocation code is expected to change from one action to another in the future.
Hmmm...
puts on complicator's gloves
class Actor { hasOneOfProps( props ){ for( let prop of props ){ if( this.hasProperty(prop) ){ return true; } } return false; } } class Action(){ revoke(postId) { /* whatever it is it does */ } isTarget(){ throw new TypeError("Not implemented. This is supposed to be a base abstract class you idiot"); } tryRevoke(){ throw new TypeError("^ What the other guy said ^"); } } class ScumTargetAction extends TargetAction { isTarget() { return true; } tryRevoke(game, actor, post){ if( game != this.game || actor != this.actor ){ throw new Error("Nuh-huh"); } if( !actor.hasOneOfProps(['scum', 'mafia']) ){ return null; // Bugger off } // Revoking time! this.revoke(post.id); return 'scum'; } } // class Scum2TargetAction { ... } // class CultTargetAction { ... } // ...you get the drift class Game { actions: [], // Assuming that's what you have function revokeTargetActions( actor, post ){ // Lower priority first const TOKEN_PRIORITIES = [ 'scum', 'scum2', 'cult' ]; // Look! We're iterating over actions only once! // (I'm resisting using Array#reduce() again) let currPriority = -1; let currToken = 'target'; for( let action of this.actions ){ if( !action.isTarget() ) continue; let token = action.tryRevoke(this, actor, post); let priority = token ? TOKEN_PRIORITIES.indexOf(token) : -1; if( priority > currPriority ){ currToken = token; currPriority = priority; } }); return currToken; } } // ... then((post) => { let actionToken = game.revokeTargetActions(actor, post); return game.registerAction(post.id, actor.username, target.username, 'target', actionToken); })To think I could have taken the time it took me to write this to play Terraria instead...
-
@Arantor said in Applying the Linus Torvalds “Good Taste” Coding Requirement (article):
@accalia figuratively speaking, of course.
no, i do mean literally.
granted they were small wars, but wars they were nevertheless.
-
@LaoC said in Applying the Linus Torvalds “Good Taste” Coding Requirement (article):
@flabdablet said in Applying the Linus Torvalds “Good Taste” Coding Requirement (article):
The compiler has worked out that the only time it needs to write back to
headis if the while loop never runs even once, so the conditional in question gets hoisted to the start of the proc amongst the jumps that set up the loop. The "optimized" version actually ends up costing an extra jump in order to force updating the header to use the same code as updating a walked-to link.Whow. Nifty indeed! I'd like to retract everything and argue for the opposite now (I hear that's en vogue here now) :)
Which kind of shows what a lot of people (including myself) have been saying here: don't bother trying to write smart code, you'll end up falling on your face more often than not. Write code that works and that you can understand.
-
I disagree with the example.
This is a case of refactoring to a very specific case. It's actually removing extensibility.
If, for some reason, you decided to make your list into a tree, or doubly linked list, then you have to throw away the code and start over.
But with the non-refactored version, you simply modify it, because it's explicit.
For example, with the non-refactored version, you simply add the extra edge cases to the top, and then you alter the navigation. With the refactored version, you have to throw away the pointer magic and develop NEW pointer magic.
INB4 people say "why do you want to change your linked list into a tree".... well, we wouldn't be coding a linked list anyway.
Scale this up to actual software you'd develop, and you can see that "Good Taste" is simply locking you into a specific implementation.
I mean, it's a good idea if you decide you're done adding features.
-
@xaade said in Applying the Linus Torvalds “Good Taste” Coding Requirement (article):
, for some reason, you decided to make your list into a tree, or doubly linked list, then you have to throw away the code and start over.
But with the non-refactored version, you simply modify it, because it's explicit.Why is it better to modify than to throw out and redo? If the requirement changed, sometimes it's cleaner to just rewrite your piece. And if you have good tests around it, you can do that easily without breaking everything else.
-
People don't want code with good taste, they want code that tastes good.
