8 ms·
I've loved John's code since I saw it first time when the original Quake was leaked from their FTP site through IP spoofing. I was just a kid at that time, and
by dchichkov 14y ago
I've loved John's code since I saw it first time when the original Quake was leaked from their FTP site through IP spoofing. I was just a kid at that time, and it was an amazing experience to hack it.
Yet now, the first example that I saw in this article hurts my eyes. Compare:
for ( i = 0; i < in->numVerts ; i++ ) {
dot = plane.Distance( in->verts[i] );
dists[i] = dot;
if ( dot < -LIGHT_CLIP_EPSILON ) {
sides[i] = SIDE_BACK;
} else if ( dot > LIGHT_CLIP_EPSILON ) {
sides[i] = SIDE_FRONT;
} else {
sides[i] = SIDE_ON;
}
counts[sides[i]]++;
}
Versus:
for(i = 0; i < in->numVerts; i++)
{
dot = plane.Distance(in->verts[i]);
dists[i] = dot;
sides[i] = dot < -LIGHT_CLIP_EPSILON ? SIDE_BACK :
dot > LIGHT_CLIP_EPSILON ? SIDE_FRONT :
SIDE_ON;
counts[sides[i]]++;
}
- Cushman 14y agoBravo. That comparison makes worrying about vertical space seem ludicrous.
- jblow 14y agoUgh. I prefer the Carmack version. This example turns the code into something that appears more airy but in fact is much harder to understand due to extensive use of ? :. I find that one of my own major steps toward programming maturity happened when I stopped doing goofy things like this and started writing code that was as simple as possible to logically follow, and that was as un-special-cased as possible. (By this latter I mean, if you change the code a little bit, you don't have to rewrite it; it looks basically the same. Imagine you want to do more than just assign one variable inside the clauses of the 'if'. In the Carmack version you just add more code there. In the proposed substitute, you have to rewrite the whole thing.)
- dchichkov 14y agoThe assignment sides[i] = ... with "? :" formatted as a table (in table notation) should be red as a single statement. It is a relatively common element, you will find in easy to read, once you see it a few hundred times.
- Garoof 14y agoI like the last one better. I think it reads more like "assign one of these values to sides[i]" and less like "do one of these three things". "By this latter I mean, if you change the code a little bit, you don't have to rewrite it; it looks basically the same." I'd say that's kind of the point though. The first one would look "basically the same" if in the last else it assigned to sides[j] instead of sides[i]. In the last one it would look very different if it was about "doing stuff" rather than choosing which value should go to sides[i].
- dchichkov 14y agoPrecisely. And there is even a little bit more to it. 1. it reads "assign one of these values to sides[i]". 2. it would not allow some other peoples spurious code into the assignment. which is a good thing. 3. space is used to convey meaning; note how sides[i] stands next to dists[i]; ternary operation is formatted as a table, etc.
- jblow 14y agoNot allowing other code in is a bad thing. See my putty reply above.
- dchichkov 14y agoIt depends. But in this particular case, some trivial extra code inserted inside that if statement may break things. Like breaking vectorization of that 'for' (note, that we are going over vertices). When you are writing the code, some times you may want to put additional constraints on the allowed operations, modifications, etc. A trivial example in C++ would be using 'const &' instead of '&'.
- jblow 14y agoThis is true (especially: breaking vectorization). But premature optimization is not a good idea, and if you are doing real optimization, you are going to rewrite that piece of code 10 times anyway, so it is in a different class of problem and the putty stuff I was saying before does not apply (i.e. this code is in the 1% or so of the codebase that is highly performance-sensitive). Optimized code is just a different thing from general code (if one is a productive programmer).
- Cushman 14y ago...really? I don't even write C that often, and I didn't find that difficult at all. Even if you don't grok the syntax yet, it just does what it seems like it does at first glance. And how on earth does a two-long ternary chain strike you as "extensive"? If you want to do something in addition to assignment, it's really quite easy: Just add a new `if` block underneath. This clearly separates your conditional assignment from whatever else is going on that may or may not be related to the assignment. (What if you add something besides assignment to the if, but then want to change the conditions for only that? You have to rewrite the whole thing!)
- georgeorwell 14y agoBasically it's a concise way to do a hard-coded table lookup. Tables are better for structurally regular code than if-else precisely because they restrict what that code can do.
- chipsy 14y agoI agree with jblow. I consider ternary a "sharp knife" in that even though it saves space, I never want to see it used more than one level deep. I don't want sharp knives in my code, I want Lego bricks. If that means the source code is a little bigger or the runtime is a little slower, that's OK, the vast majority of the time. The most egregious example of I can think of in this kind of thing is JS "gurus" who try to use the fewest semicolons possible in their unminified source. It doesn't add readability, and the semicolon rules are complicated enough that one could easily write a bug while trying to stick to the style. Sometimes languages have "tricks" that aren't sharp, and those are OK to use when you know them, but one has to distinguish.
- Too 14y agoIf / else is a way of defining what to be executed based on some condition. The ternary operator is used to guarantee assignment to a variable based on some condition. It's not only a way to save space, it's a way to express your intent clearer. It simply doesn't leave any undefined code path behind. In your case of changing the code a bit, in the carmack version of the function there's a risk that you remove the assignment to sides[i] in one case and it will later be undefined. Though i agree that the syntax is horrendous since it requires you to remember exactly what the symbols ? and : do. Compare to the much nicer and to a newbie understandable python syntax: x = y if c else z
- dchichkov 14y agoSpace is important. Think that you are not writing code, but painting a picture. And you are using space to convey meaning. You literally want to paint code, not write it. :)
- Tyr42 14y agoWell then you should work in Piet (http://www.dangermouse.net/esoteric/piet.html http://www.dangermouse.net/esoteric/piet.html) :P
- rjh29 14y agoHere's a variant of that ternary chain, used in Perl Best Practices: sides[i] = dot < -LIGHT_CLIP_EPSILON ? SIDE_BACK : dot > LIGHT_CLIP_EPSILON ? SIDE_FRONT : SIDE_ON ; This is a little cleaner if your values are longer, and has slightly nicer diffs if you add conditionals. Note that this won't work in many languages whose ternary operator precedence is different (PHP comes to mind).
- georgeorwell 14y agoSince we're playing Space Nazi, I like this better: for (int i = 0; i < in->numVerts; i++) { dot = plane.Distance (in->verts[i]); dists[i] = dot; sides[i] = dot < -LIGHT_CLIP_EPSILON ? SIDE_BACK : dot > LIGHT_CLIP_EPSILON ? SIDE_FRONT : SIDE_ON; counts[sides[i]]++; } Note that emacs will tab-align the table rows properly if you break the sides[i] = line after the = which is why I did it; not sure about vim.
- sgift 14y agoWell, let me state this: John's version is not to my preference 'cause it has { in the same line as if, but I can live with that. But YOUR version uses not only the ? operator, which should be burned with fire but it NESTS two of them together. Please, I want to die now. :( Conclusion: We have different preferences.
- bagacrap 14y agonested ternaries are the same as if-else-if continuations. You figure that out one time, and it's easy to parse forever after. The ternary operator in general eases understanding because it extracts the common bit, i.e. "sides[i] =", meaning you don't have to carefully read the contents of each and every conditional block to verify nothing else is going on besides this one assignment.
- brooksbp 14y agothe ternary operator is awesome. it should be used more often.
- neutronicus 14y agoYeah, I really like it. It's a nice little oasis of Functional style in an otherwise pretty un-Functional language.
- barrkel 14y agoThe chained ?: idiom is very common in some C codebases. Having experienced it there, I find it highly readable, but it can sometimes benefit from parentheses to clarify precedence.
- zerooneinfinity 14y agoHow would you initialize a variable that depends on the truthfulness of another without the ternary operator. You should be careful when using extremes in programming.
- Firehed 14y agoI agree that it's visually displeasing, but it's very easy to actually read and understand. The only reason I was able to quickly understand your beautifully-aligned code was because I had read the logic in the previous block, which is stupidly-easy to parse. I also (like many others) shun the use of nested ternary operators. Probably because I'm used to dealing with PHP's straight-up broken implementation. Further, I'd have split the side being determined from the sides array, as it's rather mentally taxing to figure out the increment at the end: if (dot < -LIGHT_CLIP_EPSILON) { side = SIDE_BACK; } else if ... sides[i] = side; counts[side]++;
- chj 14y agoHonestly your version is worse. ?: is cryptic, you should not use it except in extremely simple cases.
- dan00 14y ago... sides[i] = dot < -LIGHT_CLIP_EPSILON ? SIDE_BACK : dot > LIGHT_CLIP_EPSILON ? SIDE_FRONT : SIDE_ON; ...
- crntaylor 14y agoPerhaps it's because I come from a functional programming background, but I find your re-written version using the ternary operator is much easier to understand, and prettier to boot. Well done! This is a great example of "factoring out" that satisfies both the computer science and the mathematical definitions of the term. The assignment "sides[i] = " is repeated inside the if block, and it can be "factored out" using the ternary operator, just like in math: (a * x + a * y + a * z) = a * (x + y + z) I'm even tempted to say that "assignment distributes over if", but that might be pushing the analogy too far. Any further and I might start thinking about monads, so I'll stop here.