Doom 3 Source Code: Beautiful 399
jones_supa writes "Shawn McGrath, the creator of the PS3 psychedelic puzzle-racing game Dyad, takes another look at Doom 3 source code. Instead of the technical reviews of Fabien Sanglard, Shawn zooms in with emphasis purely on coding style. He gives his insights in lexical analysis, const and rigid parameters, amount of comments, spacing, templates and method names. There is also some thoughts about coming to C++ with C background and without it. Even John Carmack himself popped in to give a comment."
His Comment (Score:5, Informative)
In some ways, I still think the Quake 3 code is cleaner, as a final evolution of my C style, rather than the first iteration of my C++ style, but it may be more of a factor of the smaller total line count, or the fact that I haven’t really looked at it in a decade. I do think "good C++" is better than "good C" from a readability standpoint, all other things being equal.
I sort of meandered into C++ with Doom 3 – I was an experienced C programmer with OOP background from NeXT’s Objective-C, so I just started writing C++ without any proper study of usage and idiom. In retrospect, I very much wish I had read Effective C++ and some other material. A couple of the other programmers had prior C++ experience, but they mostly followed the stylistic choices I set.
I mistrusted templates for many years, and still use them with restraint, but I eventually decided I liked strong typing more than I disliked weird code in headers. The debate on STL is still ongoing here at Id, and gets a little spirited. Back when Doom 3 was started, using STL was almost certainly not a good call, but reasonable arguments can be made for it today, even in games.
I am a full const nazi nowadays, and I chide any programmer that doesn’t const every variable and parameter that can be.
The major evolution that is still going on for me is towards a more functional programming style, which involves unlearning a lot of old habits, and backing away from some OOP directions.
One might suggest that every good programmer, if they spend enough time improving, eventually moves toward a more functional programming style.
Re:Not the best analysis (Score:5, Informative)
the concept of the bracket placement there is to emphasise indentation over bracketing. Once you "get" the view of it, it becomes a nice thing to look at, similar to python code.
I don't use it nowadays (too many coding standards that are written for the bracketing-style) but I appreciated it when I did. There's nothing wrong with the style, so I hope that you pass any code you review written in this style and focus on the important parts like readability of the code, good naming, commenting and the like rather than subjective opinions.
Re:Else ifs - yuck (Score:2, Informative)
case statements are not faster than if-else statements. Often a case statement will be turned into a load of if-else's by the compiler anyway (and a set of if-else statements could be turned a lookup table too!)
In any case, "far faster" is not true, the machine statements generated are tiny compared to every other inefficiency in a codebase. Thinking a case statement makes your code faster is like painting your car red to improve its speed when you've got a load of heavy junk in the boot.
Re:Else ifs - yuck (Score:5, Informative)
I know some situations else-if statements are necessary, but my understanding is that case statements are far faster.
Very often rules about efficiency like this one are incorrect. Sometimes the compiler will even change things completely when you compile it. In one example, I once carefully wrote a function to only have a return statement at the end, because I (somehow) thought it would be more efficient. Then I looked at the assembly output from the compiler, only to find that the compiler had added in all the extra return statements I had so carefully avoided. After that, I just went with what was most readable.
If you really care about efficiency, there is one way to do it: you MUST time your code. Try the case statement, and time it. Then try the if statements, and time it. If you don't time it, you are just guessing and you WILL be wrong.
The case of the if statements in the article is a tricky example, because it is a range, and writing it as a switch statement would likely be a large table. Doing this could actually slow things down because it fills up the memory caches with mostly needless information. Note this can also be a problem with traditional optimizations like pre-calculated tables or loop unrolling, they can actually slow things down.
TLDR: If you want to make your code efficient, you need to time it.
Re:Else ifs - yuck (Score:5, Informative)
Often as in you've measured it, or often as in "I'm making shit up"?
A good compiler will never implement a case statement as a load of if-else's, unless the case values are sparse, or you're not optimizing.
Meanwhile, transforming a set of if-else statements into a lookup table is seldom possible unless the if-elses all compare the same integer variable to a constant. In that case, it can in theory, but almost certainly won't in practice.
Other things being equal, a switch statement with contiguous constant cases will almost always compile to faster code than the equivalent set of if-elses. And it will be far faster. Every if/else induces a branch, and mis-prediction will be severe on most of those branches, causing 10-20+ cycles of stall on modern processors. The jump table mispredicts almost always, but only once. If one arm is taken 99% of the time you can speed things up by using an if/else and then a switch, but that's a rare case.
I appreciate the fact you're responding to the idiocy of the above post, but your points are as wrong as his.
Re:Most of His Admiration Is Not Technical (Score:4, Informative)
And a note on the relative evil of comments; bad or not, well placed comments have saved me an awful lot of time when taking on maintenance of code bases in the past
Indeed. I would rather have too many comments, to the point that some are not needed, than too few, and remain confused.
Re:subjective (Score:4, Informative)
> I don't see the point of not using STL.
Code bloat, hard to debug, memory fragmentation, and no way to serialize/deserialize in a fast way.
I highly recommend ALL C++ programmers to read this doc on why EA designed and implemented their own STL version. It provides insight into the type of problems console game developers have face that the PC game developers just routinely ignore or are ignorant of.
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2007/n2271.html [open-std.org]
Re:Beautiful code but (Score:2, Informative)
John Carmack actually commented on the article saying that the code wasn't that good.
You asked for it. (Score:5, Informative)
How do you represent that as a case statement in C++? Come on, I'm waiting. Oh, that's right. You can't.
switch (dot < -LIGHT_CLIP_EPSILON ? 1 : dot > LIGHT_CLIP_EPSILON ? 2 : 0) {
case 1:
sides[i] = SIDE_BACK;
break;
case 2:
sides[i] = SIDE_FRONT;
break;
default:
sides[i] = SIDE_ON;
}
Yeah, yeah, I know, that's totally ridiculous (although I did see things as bad and worse as a CS instructor's assistant whose job it was to grade Pascal students' programming assignments back in the day - that was very interesting to say the least).
On a side note, why can't > and < characters be used in a code element? Um, that's lame, especially for a site that discusses programming so much.
Re:His Comment (Score:5, Informative)
That's a fair point, but you have to pay closer attention to what Carmack wrote:
He said functional programming style, that doesn't necessarily mean using a functional language -- the point is that he is going in the direction of functional style when writing C++, which would involve, among other things, making as much as possible immutable (hence being a "const nazi").