After having reviewed three ... suboptimal ... design documents this week, I feel I should really say something on the topic of writing up a good design.
I know most of my coworkers don't bother reading other people's design docs, so maybe you're totally justified in going through the motions and churning out total crap to satisfy "the process". But I do. I try to read every one that comes across my desk, spend some time thinking about it, and then send back the best comments I can. I spend a lot of time on it, because I consider it an important part of my job, just as important as coding is. I would hope that you're not jumping off a cliff -- but if you are, I would like the opportunity to say something when you're on the ledge, and not, say, halfway down.
And just to be clear -- we're not talking heavy-duty architecture here. We're not talking about hundred-page tomes of densely written academic arcana. The sort of design documents we're talking about are really no more than one engineer telling another, "hey, I'm about to write the Foo feature, this is how I was planning to do it; tell me what you think". They'd be just as effective if they were simple emails.
So please, I beg of you, write something I can read.
I ask not just for my own sake. Carelessness in writing correlates well with carelessness in thinking. If I can't understand what you're trying to say, I can safely conclude that not only are you a poor writer, but your idea probably sucks too. On the other side of that coin, I've never run into a well-written document describing a truly awful design idea. It's just really hard to write lucidly about something that fundamentally doesn't make any sense. Try it sometime if you don't believe me.
On that note, let me also point out that design review meetings are for REVIEWING DESIGN. It is not for explaining your poor chicken-scratchings of a design document. A random brain dump of disorganized ideas does not a design document make. Sorry. If I can't get a clear idea of what you intend to do from the doc alone, you wrote it poorly; go back and fix it before you schedule a meeting. And this practice of sending out a draft of the design document two hours before the design meeting? Stop it, please, or I will have to hurt someone.
I shouldn't have to go to your review meeting. I usually don't, actually. Some people prefer giving feedback face-to-face, and that's fine, but I actually prefer to use email in order to avoid subtle miscommunication.
And on the same note: again, design review meetings are for REVIEWING DESIGN. It is not for creating new design on the fly. Sure, I know it's tempting -- you're rushed for time, you've got a dozen open issues to close on by this afternoon, and fractious debate just isn't your thing. You don't have the energy to come up with your own solutions and defend them in the meeting. So hell ... why not just leave them all open issues? Surely a gaggle of non-experts can come to a consensus for each and every issue in under 60 minutes.
Again, this is total nonsense, and it needs to end. Don't leave open issues hanging. Coming up with solutions is your job, telling you if they suck or not is my job.
Why Design Documents?
I'm not a Big Design Up Front guy. My philosophy is A Little Thinking Up Front. I distrust TDD as a methodology because it strikes me as too much random-walking the problem space to stumble on an acceptable solution. But I certainly don't condone overthinking the problem either.
To be honest, I actually don't think design documents have much value in and of themselves. I subscribe to the Dwight D. Eisenhower school, where plans are worthless, but planning is everything. A design document should be well-written, not because someone needs to read it, but because the writer needs to be thinking precisely and rigorously if they have any hope of creating a precise and rigorous design.
The end result, however will always be subject to change in light of changing requirements. Contrary to some people I know, I don't see the point of keeping the design document up to date. It violates DRY. The documentation and the code will inevitibly drift apart despite your greatest efforts; it's really not that hard to understand well-written code in the first place, so why not go to the authoritative source directly?
It's a truism that no one reads design documents anyways; if they do read them, it is as a historical artifact -- to get an idea of what one person was thinking at one point in time, and not necessarily what the design is now and eternally should ever be.
On Document Templates
Finally, a word on document templates. Yes, we have a standard document template in my group, and no, it doesn't do a damned bit of good.
Our template is really quite simple. It has four main sections:
- Introduction
- High-level design
- Detailed design
- Other considerations (a dozen items such as localization, performance, security, error handling, etc).
Most people get through the introduction OK. But the distinction between "high level design" and "detailed design" is a total fucking mystery to most folks -- they sort of run into each other in one long ramble. Personally I usually rename them to "interface" and "implementation" respectively: the theory being that once an interface is nailed down, the implementation is pretty unimportant; 90% of the time it practically writes itself.
And no one, but NO ONE, ever fills out the "other considerations" section of the template. Neither does anyone hold anyone else accountable for completing it. Let this be a lesson for document template writers everywhere: you can lead an engineer to water, but you can't make him think.
One of my favorite interview questions is "how do write design documents?" At the risk of tipping off a potential interview candidate to what I think the ideal answer is, here's the template I'd prefer to be using:
- Introduction: what are the background, goals, relevant functional specs, etc?
- Class diagram: who are the major players? What are their responsibilities? What state do they carry?
- Interfaces: tell me about all new public functions, classes and/or interfaces you plan to define. Don't describe private methods and other implementation details: save it for an appendix if you REALLY think it's important to write down.
- Examples: Show me sample code, test code, or sequence diagrams that illustrate how these new interfaces are intended to be used.
- Defense: what other ideas did you consider? What are their pros and cons relative to the design you eventually chose?
- Test cases: are there any tricky parts you think might be difficult to get working correctly? If so, explain ways you can test your design for correctness.
This weekend I got 84th place in the online 10th annual International Conference on Functional Programming contest. Not bad, given that I hadn't any intention in, you know, actually competing.
The task: in this year's competition, you are given 7MB string of characters representing "alien DNA". Like human DNA, each character represents one of four bases -- but instead of A, T, C, G, the four bases are I, C, F and P.
You are also given detailed instructions to decode the alien DNA. You can implement this any language you want, using any tools you deem necessary. Essentially, bases are consumed from the left end of the string, and the consumed bases indicate how to mutate the remainder of the string, as well as generating a series of 7-character strings called "RNA". The process is iterated until the DNA string is completely consumed.
The series of RNA strings represent drawing instructions. "Executing" the RNA should result in the image of an unhappy alien and a crashed spaceship.
And if you are able to get to this point, the puzzle has just begun ...
The real object of the competition is not to generate the image of an unhappy alien, but to "morph" the image into a happy alien and a fixed spaceship. This is done by prepending a "prefix" to the DNA string, which alters how the rest of the (self-modifying) DNA is interpreted.
Oh, and did I mention? You only have 72 hours to write this all up.
As you can imagine, it's as hard as hell. But also extremely fun!
Day 1 (Friday): I've never done an ICFP contest before, so I had no idea what I was in for. I had learned about it from reddit earlier on in the week, and even briefly glanced at the contest materials on Friday when they were unveiled, but never gave much thought into actually joining the quest. But when I got home from work on Friday evening, I saw I had a few hours to kill, and what the hell, why not at least do a little bit and see how far I get?
And so I begin the long, slow slide into the abyss ...
The coding was very straightforward, translating pseudocode into C++. I could have used Java or C#, but I'm at the point where programming at a high level in C++ is just as easy, and I wanted to retain the option of quickly dropping down into some low-level bit-twiddling if it came down to it. Erlang is my other favorite language for hacking around in, but was worried about running into some performance problem that I couldn't extricate myself from.
By the end of the night (i.e., 2am), I had a mostly-working DNA-to-RNA translator. But it was awfully slow. According to the contest materials, complete execution of the DNA requires 1.9 million iterations -- and so even at 100 iterations a second, I calculated it would take around 6 hours to finish.
Day 2 (Saturday): I woke uncharacteristically early on Saturday morning and got immediately to the business of writing a faster interpreter. Originally I had a used a std::deque to represent the DNA, owing to the frequent case of removing bases from the front of the string. But much of the time was spent copying sequences of DNA megabytes in length. It stands to reason that copying a million bytes a million times would take an extremely long time. The contest materials even suggest that "It seems particularly important to ensure that appending unquoted references perform better than in linear time".
My second attempt was to replace the std::deque with a std::list of characters, so that I could used the splice( ) method to combine subsequences. I didn't get too far down that path before the problems became obvious. First, not all subsequences were splice()-able -- some would need to be duplicated, and recognizing those situations added complexity. But an even more grave problem was that the common case of skipping down the list by n bytes went from O(1) to O(n). I wasn't making progress; I was only trading one problem for another.
I quickly abandoned that train of thought and went to write my own container. At first I started writing my own deque, implementing it with a std::vector under the hood. After all, in the usual case only a few hundred bases would be consumed, and then the string would be thrown out and replaced with a mutated string. It wasn't necessary to aggressively reclaim the memory the way that deque does. This improved performance a great deal -- instead of a ridiculously-long 6 hour runtime, I was looking at a ridiculously-long 2 hour runtime.
My fourth try was a ugly mess (I was getting tired at this point, and my code was getting sloppy). The idea was that I would represent only the diffs between DNAn and DNAn+1 -- and then chain this information together. To determine the 1000th character of iteration 10, I'd find where that character was in iteration 9, which entailed finding its location in iteration 8, etc. etc., until I found its location in iteration 0, where the data is actually stored. Every 100 iterations or so I would renormalize this structure so that the number of pointers I was chasing wouldn't grow without bound. But needless to say, the performance sucked for just about every use case.
I finally got it right the fifth time around. Idea #4 wasn't a horrible idea -- the only poor decision I made was to keep 100 iterations worth of history around. The truth is, I only needed to keep one buffer of DNA, and one list of "blocks" with indicies into that buffer.
At 2am in the morning, my DNA-to-RNA translator weighed in at 530 lines of code and took about 3 minutes to run.
Day 3 (Sunday): my wife and I ran a few errands in the early afternoon, so I didn't get back to coding until around 5pm. At this point only eight hours remained in the competition -- and I hadn't even gotten to the second half, which was to convert the RNA into a picture! Even if I got that part working, I would only have a picture of an unhappy alien to show for it. I was completely at a loss for how to gain points by "morphing" the image into something even vaguely resembling a happy alien.
Fortunately the RNA-to-picture converter was much easier than the DNA-to-RNA converter. Three hours and 300 lines of straightforward coding later, I had it working flawlessly. There was only one bug in the whole thing, which didn't take too long to ferret out and fix. The converter dumped out the final image in 24-bit uncompressed BMP format -- it was the simplest thing I could come up with at the time, and I figured I could always just use GIMP to translate to a better format if need be.
The contest materials indicate that "something curious happens if the prefix IIPIFFCPICICIICPIICIPPPICIIC is used". That "something curious" turned out to be a self-check page (which turned up the only one bug remaining in my DNA-to-RNA converter, a corner case involving a out-of-bounds index).
That leaves five hours left to figure out how to morph the image ...
My first idea was to start with the image background. It was relatively uncomplicated -- all I'd have to do is lighten it up a bit. That shouldn't be too hard -- I could just fix that part and then call it a night. All I'd have to do is find the part of the DNA that dealt with drawing the background, change some constants, and voila ...
But first I'd have to find out where that DNA was.
I inserted some code to dump out intermediate results from the RNA-to-picture converter. And that's when I noticed it -- a string of DNA letters, hidden in the first few drawing instructions, which is immediately overwritten by the "unhappy alien" image. I punched in that string, reran the process, and out popped a secret message from the contest creators. Something to the effect of "it looks like you're trying to morph an image. We don't recommend you do it yourself, but if you absolutely have to, here's some technical details about how to do it".
It then gave two DNA prefixes. One of the prefixes generated an "almost there" image -- the nighttime scene had turned to day, but many details were missing. I assumed this was a "checkpoint" that you could submit and get credit for at least getting this far. It also explained why a number of contestants had all submitted the same 28-byte answer.
I quickly registered a new account and sent in my discovery. Three and a half hours left in the contest, and I was finally on the board, at position 151 (of 860+)! It turns out that a great number of contestants hadn't even gotten this far -- and from looking at the top of scoreboard, it seems other teams had gotten a lot further than I had.
(Later on I discovered that the 28-character freebie was one character longer than it needed to be. Other contestants had submitted 27-byte entries which, because it was one byte shorter, scored one point higher than the others. I figured out this trick, which moved me up to 84th place).
Back to the secret message. The other prefix turned out to generate another secret message: "navigating the field repair guide". Unfortunately, this page contained only cryptic instructions and the number "1337". A lot of trial-and-error later I finally was able to figure out what the instructions were getting at, and I was able to generate the "table of contents" to the "field repair guide".
By 2am I had most of the "field repair guide" printed out. Some of it was helpful, but other pages were amusing joke pages. The repair guide seemed to indicate the string-replacement DNA system was a fully Turing-complete programming language in itself. The middle section, which was never modified, contained subroutines ("genes") which would be copied to the front of the string (where they would be executed). The end of the string appears to be a temporary "stack" containing intermediate values, parameters and return values.
By this point, however, I realized that with one hour left in the competition, I was not likely to learn anything that would move my score any higher -- and seeing that I had to be at work Monday morning, finally chose to bag it for the night.
Conclusion: if I had to do it all over again, there's a few things I'd do differently.
- I'd print out the spec: if there was anything that hampered my productivity, it was in endlessly scrolling around in an online PDF.
- I'd read the spec more carefully: you know, after having printed it out. I noticed early on the warning that about skip and append needing to be fast, but it never sank in until about halfway through that they both need to be sub-linear time. Being fast and linear just wasn't going to cut it. If I had recognized that before coding I would have probably jumped directly to the buffer-and-blocks representation, rather than stumbling upon it after four failed mistries.
- I'd involve other people: next year I am so going to organize a team. This would have been so fun to work on with other people. Plus, given that the DNA-to-RNA and RNA-to-picture converters were two separate programs, we could have worked on them in parallel -- leaving more time for the last part, the actual morphing, which I never got around to.
- I'd have blocked out the time in advance: when I get immersed in a problem, interruptions become intolerable. Squeezing in time between strange looks from my wife (what, you're STILL on that computer!?) just doesn't cut it. I need time to concentrate!
Other post-mortems:
The other day, I was reviewing a coworker's design for an area she was writing. As she was describing the responsibilities of each class, my attention was drawn to one in particular:
"Why is this method in this class Foo? I mean, it's true that the operation does have something to do with Foo objects in general. But isn't a really specific method, one you'd expect the user of this object to be doing? It's a bit if you were to have this really app-specific string manipulation function, and then throwing it into the global string class even though no one else would ever need it".
Her response:
"Well, yes, I can see that point. But you're always saying that we should write small classes, and well, the calling class is already quite big. So I thought I would spread the complexity around a bit".
That got me to thinking. Because it is true -- I am always going on about "one class, one responsibility" and the dangers of monolithic classes. But in this situation, the "fix" struck me as a bit naive. It's a bit like how everyone knows that long functions are bad, but if you have a bit of code that genuinely does call for a long function (for example, some function that does step A, then step B, then step C ... all the way to step Z, in a very procedural fashion), the common "solution" is to chop it up into a bunch of single-purpose functions.
But it's hard to say that that's any improvement. What was once straightforward to read now requires jumping around in the editor. And now it's not at all clear that these dozens of (frequently poorly named) funclets are only called by that once-ginormous "DoStuff" function. At best you've gotten control over all of the local variables, but anonymous scopes could have solved that problem.
There are a million ways a program can be written, but only a subset of those are agile, maintainable designs. How do you go about finding them?
When I was younger I learned that most sheets of paper have a sort of "grain" to them. You can't notice it by looking, but there's a simple test: rip the same piece of paper vertically, then horizontally. One of the two will be a nice, clean, straight tear, but the other will be an irregular, raggedy mess. In the same way, any given program has a "grain", an underlying structure that dictates an optimal factorization into classes and functions. That underlying structure may not be immediately apparent to the naked eye -- but you'll know soon enough the moment you start "coding against the grain". To avoid ending up with a big ball of mud, a programmer has to listen carefully to find out how the program "wants" to be written.
Here are some common rookie mistakes I see in many newbie designs that indicate to me that someone's been coding against the grain:
No Functions, Just Objects
It's a pity that the plain-ole' function has been all but forgotten due to the object orientation craze of the 90s. Some mainstream languages like Java and C# pretend flat functions don't even exist (although .Net 2.0's static class concept has rehabilitated their image somewhat).
The contracts of functions are simpler and easier to express than the contracts of objects. I've never seen this exact code, but I've seen the spirit of it many times:
class Multipier_t
{
public:
void setX(int x);
void setY(int y);
void doCalculation();
int getResult();
private:
int m_x;
int m_y;
int m_result;
};
Factory functions seem to suffer from this anti-pattern the most. For some odd reason factory method I've ever seen came swaddled in a do-nothing class -- even in C++, where it's unnecessary. It's almost as if it were called the Factory Class pattern and not the Factory Method pattern.
Every application is different and the exact ratio varies, but it shouldn't be considered unusual when half the code is plain old functions and the other half is regular OO code with classes.
Paramaphobia
A similar pathology is the fear of function parameters. You've been there, I'm sure. It starts innocently with one or two arguments, and before you know it, you're hurtling down that slippery slope. Is pszSecurityParams the eighth or ninth parameter to FrobnicateFoo? Ah, to hell with it, let's just make them all takes-void-returns-void and throw them into the same class, where they can share information with impunity.
And then we're back to that happy sunny world of global variables (hello, world!). A rabid outreak of function parameters is no good thing, but there are far better cures for this disease (such as Introduce Parameter Object). A function that take many parameters is probably doing too much, anyways.
Or perhaps you're simply in the mindset of, "hey, these two functions are in the same class. Why do I need to pass parameters that could be fished out of the this parameter?" The answer: referential transparency. Without parameters, you can't readily see what the called function uses as input, what it modifies and what it returns as output.
Objects should only carry around information that must persist between invocations and which can't be easily recalculated.
In my own code, the vast majority of functions neither rely on nor generate side effects -- they are strictly dependent on their input parameters only. This style makes writing tests and refactoring code hella easier, and I cannot overestimate the amount of simplicity it buys me.
No Mapping Between Responsibilities And Objects
Violating the Single Responsibility Rule is another way to get into the same "global variable" mess. A class with two responsibilities is a blatant invitation for one responsibility to interfere with the other. And a class that implements half a responsibility isn't much better: it's constantly communicating with some other object in the system to get its own work done.
Sometimes it's hard to nail down exactly what constitutes a responsibility. After all, a class is constructed of many methods, each of which should also have a single responsibility (albeit a more limited one). Neither can it be seen easily on a UML diagram: does that single line between two boxes represent a simple interaction or an incestuous coupling? It's hard to say.
At such times, it's important to listen to the code and keep an open mind. All programmers come up with mental models of how problems should be decomposed. Some of those models are accurate. Most are not. The difference between a good designer and a bad one (other than coming up with the right model in the first place) is that a good designer won't rigidly impose their beliefs on reality when they don't fit. Rather they adjust their model based on what the code is telling them, letting the shape of the code dictate who the real actors are and how they co-ordinate together.
No Objects, Just Subjects
On the Portland Pattern Repository wiki there is a page on a proposed "best practice" entitled: "Dont Name Classes Object, Manager, Handler, or Data". Sure, it's somewhat tongue-in-cheek, but I think they may be on to something.
In English, most sentences have a subject, an object, and a verb. The "verb" is that venerable workhorse, the function, and the object is usually ... some object. But what's the subject map to? Actually, the subject is usually not important. Often it's some fictional construct, such as "Handler" or "Manager" or -- dear Lord, "Wrapper" (shudder). Empty words such as these can always be omitted.
Stay away from the Kingdom of Nouns, and challenge any class that ends in -er. Functions do things. Objects have things done to them.
No Distinction Between the Necessarily True and the Contingently True
Some time back I was tracking down a memory corruption. The code looked like this:
void OnNotification(int type, void* pNotification)
{
if (type == DB_NOTIFICATION)
{
((DbNotification_t*)pNotification)->fHandled = true;
}
else // it's a UI_NOTIFICATION
{
((UiNotification_t*)pNotification)->fHandled = true;
}
}
At the time the code was written, the comment was entirely correct. There were only two types of notifications, so if it wasn't one, it had to be the other. And the code worked for a good long while. But when the third type of notification was introduced into the system, all hell broke loose. This is a good example of the difference between something that is "contingently true" -- something that just so happens to be true for the moment, but not rigidly true in the same sense that "A or not A" necessarily has to be true according to the laws of Boolean algebra.
Newbies muddle this up all the time. You can see it, for example, with hard-coded numbers that assume that the display size will always be 800 x 600, or the same mathematical tax-calculation formula duplicated in various locations throught the code.
A key software principle is "alternate soft and hard layers". What this means is that a good design makes an attempt to separate things that are highly unlikely to change from the things most susceptible to change. If successful, you can end up having a large amount of code that, once written, needs to be looked at rarely if not never again. Your energy can be better spent on the small bits of code that do need changing in light of rapidly changing requirements.
It's midyear career discussion time again at Microsoft, and you know what that means: more sausage!
More specifically, "sausage, sausage, sausage" is the phrase I mutter to myself every time I'm dragged into yet another meeting explaining the performance review process. The meaning is this: the performance review system has all the appeal of watching sausage being made. While I find it endlessly fascinating to learn what goes into the sausage, and while I'm glad Microsoft HR is committed to explaining every detail of how the sausage is made, in the end ... it's all just sausage.
Microsoft officially has a "pay for performance" philosophy. At least that's what they claim on the offer letter every new employee receives. What they don't tell you is that the philosophy and the implementation don't match. The implementation is closer to "pay for perceived performance measured relative to your immediate peers who are working on uncomparably different things than you which may be less sexy but no less vital to the project's success, divorced from the group's actual results measured subjectively by people who sometimes have no idea what you do from day to day".
Ultimately, the system isn't fair or rational and the review score you get has absolutely nothing to do with your potential or your performance.
None of this is sour grapes, by the way. I've been fortunate so far; I've got nothing to complain about. I am simply pointing out a universally accepted, if unspoken dirty little secret here at MSFT: your review can suffer for any number of reasons unrelated to you. Maybe you weren't set up for success by your manager, your team or your PMs. Or your work wasn't "visible" enough. Or that you're simply in a good group where there's tough competition for limited, yet differentiated awards.
Objectively measuring people's performance is a hard problem. No matter what system you can imagine, your performance review will invariably be influenced highly by what your immediate manager thinks of you. If you have a poor manager, you're screwed; he'll never understand your true worth -- unless, of course, you're a very talented bullshit artist, in which case it's someone else who's screwed, maybe the person doing the actual work.
There is really no substitute for a good management chain. But neither is there any guarantee of good management either. Groups at Microsoft run the gamut from ridiculously amateurish to amazingly productive. Yet you can't tell from results alone which is which. Some groups are somehow always the victims of forces outside their control, whereas other groups could have easily smashed through the same obstacles. And yet other groups truly are confronted with harsh realities which cannot be humanly overcome.
Incompetent groups can go on malingering for years before anyone finally realizes what's really going on.
So I'd rather not watch the sausage as it's made. I simply don't care about levels and CSPs and proficiencies and commitments and experiences and contribution ratings and stack rankings and all this other nonsense. I guess I should, because there's good money in it if you can learn to work the system. But does it all really matter? Not really ...
Such complicated sausage-making, and yet the outcome is ironically no different than this: do your best, stretch yourself to new limits, cross your fingers and if there's any justice in the world, you'll get what's coming to you.
For the really curious ... step up and smell the sausage ...
I suspect my coworkers may consider me mildly insane.
In our group, we typically require that code must be seen and reviewed by at least one other person before checking in. I try to do my part, and tend to respond to a lot of code reviews that come across my desk. The thing is, I almost never review for algorithmic correctness. My coworkers are fairly bright people, and honest enough to check that something actually works before sending out the code review. Nor am I much of a coding guidelines nazi. Actually we have some fairly ridiculous guidlines still hanging about; nearly everything that makes C++ different from C is considered somewhat esoteric, obscure, and prone to abuse. I don't share in this particular form of language bigotry, so I don't bother to point out such violations.
Rather, my reviews tend to focus on issues of design, structure, and readability. That's right, I'm the annoying git who sends you dozens of suggestions where a public method could have been private, or a variable declaration scoped inside a loop, or the use of std::auto_ptr could have saved you from manually coding up an object dtor, or that you should have moved an if statement from the caller to the callee, or that you duplicated five lines that ought to be pulled out into a separate function, or that you've added a superfluous member variable that really ought to be passed around as a function parameter. All these ridiculous, nitpicky, useless suggestions ...
Or are they so useless? The thing is, I have a bit of a reputation in my group for being able to jump into unfamiliar code and understand with unusual speed the overall design. And as I think about it, I don't think it's because my brain neurons fire faster than anyone elses'. It's just that I look at different things than they do.
Suppose that you've suddenly been dumped into a project with tens of thousands of lines of C++ code, and you need to come up to speed on it yesterday. Obviously you can't read it all in one day. You've got to start somewhere. You've got to simplify the task somehow. But how?
Do you jump into the code? Find the biggest function you can find and start tracing through it -- reasoning being that most of the interesting code must be in the longest functions? Good luck with that. Maybe you go scavaging among the function header comments. If so ... you poor misguided fool. Don't you know that function header comments only come in three flavors?
- Comments that are blank templates no one ever filled out
- Comments that are two years out of date with the code
- Comments that are gee-duh rephrasings of the function name and parameter list.
Ignore what your college professor told you; function header comments are useless. Don't even bother reading them. In fact, try to avoid writing them, except in cases where they're absolutely necessary.
Let me share a simple trick with you. Ignore the CPP files. Look at the headers only. In fact, don't look at all of the headers, just look at the classes. And don't look at the entire class - ignore all the private methods. The secret is that nearly everything you really need to know about a class can be inferred from the public methods it supports and the private fields it contains. And if there's a comment there explaining the class's role in life, you're really in business.
Back in the Structured Programming Era (back in the Jurassic age, I believe), Fred Brooks observed, "show me your flowchart and conceal your tables, and I shall continue to be mystified; show me your tables and I won't need your flowchat, it will be obvious". In modern terms: understand the underlying data structures, and the algorithms can be inferred. Once you know what's in an object, and especially what operations the outside world can do on the object, the actual method implementations write themselves.
For this reason, class diagrams are always far more interesting than sequence diagrams.
If you start doing this, then you actually start caring about whether an object has too much state in it, or whether it has too many public methods. They become a distraction. It actually makes a difference if you use std::auto_ptr to define a member variable, as opposed to a plain old pointer -- std::auto_ptr explicitly documents that the object is contained rather than shared, which is enormously useful information to know. And you finally stop that amazingly annoying knee-jerk habit of declaring every class and every function in a .H file, regardless if used outside of one .CPP file or not.
To be sure, if the code is genuinely a rat's nest of dependencies, no technique in the world is going to help you. But more often, deep within that mess of code you've inherited, there's some semblance of structure there struggling to break free. With practice you can learn to discern the patterns which indicate that underlying structure.
And, I think, you become a better programmer because of it.
Then Philip ran up to the chariot and heard the man reading Isaiah the prophet. "Do you understand what you are reading?" Philip asked. "How can I," he said, "unless someone explains it to me?" (Acts 8:30-31, NIV).
A couple days ago a coworker lamented the fact that the Design Pattern training course was booked for six months solid. Oh no, how horrible. Guess you're just gonna have to go on being ignorant of design patterns until the middle next year. Can't possibly understand something unless someone behind a podium explains it to ya ...
Dude, it's not like they're going to cover anything you couldn't just read in the Gang of Four book. I'd be willing to lay good money they're not going to say anything interesting, like how design patterns are an incomplete solution, that when you find yourself doing the same thing over and over again, the right thing to do is find better tools, instead of laboriously cranking out the drudge work by hand; like Steve Yegge says, a good third of the GoF design patterns are things that programmers in functional languages could do in one or two lines and they would never dream of inventing cutesy names for such obvious things ...
Naw, it's probably gonna be three hours of: "Here is the Bridge Pattern. It looks like this in UML. You use it in these situations. Click, next slide. Here is the Decorator Pattern. It looks like this in UML. You use it in these situations. Click, next slide. Here is the Singleton Pattern. It looks like this in UML. You should avoid using this pattern if you can. It's a fuckin' global variable is all it is. Click, next slide ..."
I had a similar reaction when another coworker suggested we videotape already existing documentation. Because, um, it's easier to have someone explain how to do something, rather than read how to do it in some dry, dusty webpage or Word document.
Deep breath here. Okay, the documentation could be improved. Fine. And sure, different people have different learning styles. I get that. Maybe there's something about seeing a face or hearing a voice which makes the subject material more engaging and more interesting to listen to. Perhaps the human presence is what opens up pathways in the brain that reading the written word is unable to accomplish. Learning becomes more a form of entertainment. I'm positive at the neurological level, that's exactly what's happening.
But gawd, is it inefficient.
I don't mean that reading is a higher bandwidth form of communication than listening. No, humans comprehend speech at a rate of around 200 words a minute, which is coincidentally almost the same speed at which the average adult can read. Of course that rate can vary -- for extemporaneous presentations of technical material, you can count on the speaker umming and ahing at around half the rate of normal conversation.
But that's small potatoes. So I can read your slides in about half the time it takes for you to read them to me. So what. What really bothers me is when you, the presenter, begin to cover material I already know. I can't blame you, of course -- after all, it is a mixed audience, and you can never count on everyone having been exposed to the exact same prerequisite foreknowledge. But when a writer begins to belabor the obvious, I can always skip a few pages. I have no remedy when a real-life presenter does the same. It's just another 15 minutes of my life I'll never get back. Even on video, cueing up the appropriate segment is a major hassle; it doesn't compare to how fast I can scan a written page. With written documentation, if I want to speed up, if I need to slow down -- I'm in control of the speed of the conversation.
I guess I've always been a bit of an autodidact, and that's where my distaste for formalized education comes from. It just seems to me that "training" is an entirely inappropriate word that goes on in our business. Dogs are trained. McDonald's employees are trained. "Training" means teaching a rote task in which mastery can be acheived in a few short hours. Believe me, when you come out of that classroom, you'll have only just begun to learn design patterns or whatever. You've got the rest of your professional career to get good at 'em. What you're really looking for, what you really want is an education, not a training -- and that's not something someone can ever "give" you.
Training sucks. Read a book.
I'm ashamed to admit it now, but for about eight years of my professional career, I went around saying that I knew C++, when I really had no clue.
In my defense, I wasn't exactly lying -- "lying" implies intentional deception when one knows the truth all along. Truth is, I didn't know what I didn't know. I thought I knew C++ -- turns out, I was just programming in C, with occasional sprinklings of the class and virtual keywords.
But I didn't really "get it" until a couple years ago.
You may have heard that C++ is a language which allows you to define data types that act just like native types. I certainly did. I saw that ubiquitous complex number class example, where operator+ and operator* were redefined to do the right thing. Steve Yegge sums up my response: "Wow. He's right. It is useful".
Then I forgot all about it and went back to programming in very C-like C++.
Before I get too much into it, I should explain what's wrong with C. The problem with C is that it's a very low-level language. It's been described before as "portable assembly language", which is absolutely correct. Now sure, some people LIKE having that level of power. To be honest, I do too sometimes. But most of the time I'd rather cut down trees with a dull butter knife than to program at that level of detail.
In particular, C is missing one important feature that every other language has built in: the ability to model a 1:n relationship, where n has no fixed upper bound. In C, the only you tool you have is a fixed-sized array, and that's it. Every other language has lists or arraylists built into the language. Heck, in any other language strings are basic types, and after all, a string is just an ordered collection of characters.
And so, in C, if you want to deal with strings of arbitrary length, linked lists, or any other 1:n relationship, you've signed yourself up to write reams of code to allocate, copy, and free structures and to resize arrays. And you've got to be so careful that when you add a new field to a structure, that all this boilerplate code is correctly written. It's extremely tedious.
Which gets me back to the point about being able to define data types in C++ that act just like built in types. Up until I saw this line of code, I really had no clue the limits of what's possible:
std::string s2 = s1;
When I saw this, I was shocked that I could subsequently modify s1 without concern about s2, because strings had value semantics (aka copy semantics), not reference semantics. It worked just like integers ... if I assigned i2 to i1 and then changed i1, there's no reason why i2 should change. If you told me the same could be true for strings, I would just stare at you like you grew antennae on your head, because I couldn't possibly think of strings as ANYTHING ELSE but pointers to fixed-sized arrays of char.
No one ever explained it to me quite like that, which is why I went around eight years doing it all wrong.
There was a whole host of things I didn't know about C++ before I learned the STL:
- I didn't know destructors were called automatically when local variables go out of scope. I thought they only happened when you deleted a pointer to a new'd up object.
- I didn't know C++ automatically generates copy constructors and assignment operators for you, if you don't define them yourself. Not only that, it does the right thing. If you define a "Contact" class with a string for a name, and a vector<string> as a list of email addresses, "Contact c1 = c2" makes a copy of everything.
- I didn't know that C++ automatically calls the destructors for the member variables of a class at the end of the containing class's destructor.
- I didn't know you could return objects from a function. I thought you could only returns native types, or pointers, nothing else.
- When I learned about exceptions, I thought you always had to write reams of cleanup code in the catch clause. I didn't know about RAII.
- I didn't realize just how useful templates were. I always thought they were nigh-useless: I couldn't imagine myself ever writing a template class. I still can't, actually. But no one ever pointed out the utility of using a small set of vital, standard template classes like vector, list and string.
In short, I didn't understand how THE WHOLE LANGUAGE conspires to do the right thing, if only you'd stop thinking of it as a glorified C. It's actually quite elegant, in its own way.
These days, I can write a several-thousand line application and not use the delete keyword once, nor use a single pointer in the entire codebase. Considering that pointers and manual memory management are the two most bug-ridden features of C, I consider that a huge accomplishment. I estimate that I write about half the lines of code that I used to.
These days, I consider "do you know the STL?" as a litmus test for whether someone knows C++. Some have said to me that this is unfair, that the STL is "just a library". Wrong. It is the library. I would no sooner hire a C programmer that never heard of printf than I would hire a C++ programmer that never heard of std::vector. I'm not asking that they memorize all of printf's specification flags, or be able to give a stirring rendition of what bind2nd is and why you'd ever want to use it -- but there's a certain level of basic knowledge that a C++ programmer absolutely must have.
When I began learning Erlang, the thing that intrigued me most is this wholesale rejection of the concept of "state", "side effects", or even "loops". As I got to thinking about it, the hardest programming problems I've encountered almost always had too much state (be it local state, object state, or global state), too many convoluted loops, too many special cases, too much spooky action-at-a-distance caused by side effects, and too much duplicated, inconsistent state.
Well, actually Erlang still has loops, cleverly hidden as tail-recursive functions, or else calls to map and foldl. And Erlang still has state -- in the form of a list of parameters that gets repeatedly passed to a tail-recursive function. It's impossible to avoid side-effects completely -- after all, programming is all about creating side effects. But minimization of side effects is a noble goal.
Same thing applies with other aspects of programming, like coupling or local state. A program that has zero coupling between its components is by definition a program that doesn't work. Still, we always endeavor to remove needless coupling where it can be found. I agree with Steve, Reg, even Ken Thompson who said that he doesn't care about the length of a procedure, as long as there aren't too many local variables or too many levels of indentation. In my designs, I try to minimize state (local, object, and global), side effects, if statements, loops, and dependencies. And I find myself replacing temps with queries at nearly every opportunity.
Object-oriented programming had a big impact in the 80s and 90s, in that it taught programmers how to partition and hide state in these little bloblets called "classes". But this was only a partial mitigation to the problem of exponentially exploding complexity. Functional programming goes one step further, and teaches programmers to totally remove unnecessary state. Functional programming is the real "extreme programming". It takes all these good ideas that people have known for the longest time, and turns each knob up to 11 on all of them.
If you haven't taught yourself a functional programming oriented language (e.g., Haskell, Lisp, Erlang, etc.), you should take any opportunity to do so. Maybe you'll never use whatever esoteric language you chose to learn, but it will give you another perspective on programming, and maybe even open up another world for you in some of these mixed-paradigm languages like C# and Ruby, which have all these features that you may not have ever used.
I see this all the time:
HRESULT Foo::get_Bar(Bar** ppBar)
{
ASSERT(ppBar != NULL);
*ppBar = m_pBar;
return S_OK;
}
Eh, what's the point of that ASSERT there? Without it, the subsequent assignment to *ppBar will cause an exception which will almost invariably pop you right into the debugger at the exact point of failure. It seems redundant. The only use is that you don't see the need for real error handling, but the dialog that the OS throws up for Access Violation is just too scary for users to see (and somehow, the ASSERT box isn't).
You could say it has some value as self-documenting code. But I think it's already clear that if you wanted to get a Bar object, you had better give get_Bar a place to put it.
Eh, but that's a mild form of abuse; it just bugs me because it's a sign of mechanical thinking, a ritual done without a purpose. But nothing bad happens; at worst you've just added an extra line of clutter. What really sucks is THIS nonsense, also far too common:
HRESULT hr = ReadInitializationFile( );
ASSERT(SUCCEEDED(hr));
Can ReadInitializationFile fail? Sure it can! It could be malformed or nonexistent. What happens if this is the case? Well, some hapless developer gets popped into the debugger, they investigate the problem for the better part of an hour, and at the end of it, they make no change to the code -- there's no bug there, just a malformed INI file. They learn to shrug their shoulders and hit F5 or whatever key it is to continue execution. Five minutes later, BAM!, there goes another assertion ... no worry, it's just that bogus one again. Ignore it ...
Get it? You've done your part to train a whole generation of programmers to ignore assertions. Worse yet, you're not even trying to handle the failure in a graceful way.
Just for review: an assert should always indicate a programming error. Assert is not a logging facility. If what you need is a logging facility, there are far better alternatives. An assert should not be thrown for conditions which, however unusual or exceptional, your program handles correctly (or should handle correctly). Instead, an assert should always be a precondition, postcondition, or invariant of the code which should always be true, no matter what data is thrown at it for input, or whatever unusual (valid) code paths taken to get there. For example:
// Finds all the prime factors for the given input. Assumes
// the input has already been error-checked (ie, it's greater than one).
vector<int> FindPrimeFactors(int n)
{
assert(n > 1);
vector<int> ans;
// Many lines of code later ...
// Verify that the factors multiply up to the original value.
assert(accumulate(ans.begin( ), ans.end( ), 1, multiplies<int>( )) == n);
return ans;
}
Here's another thing that gets under my skin: the term "unit test". Not that I have anything against them, they can be a fine tool (and at some future date I'll flesh out my developer test philosophy a bit more). No, what bothers me is that I see and hear the term misused all the time, and it causes a lot of confusion.
Where I work, developers frequently claim to have written "unit tests" for their code, when really they haven't. More often than not, what they've really written is an integration test. Don't get me wrong -- both kinds of tests are very useful, but they are not synonyms for each other. It's important to distinguish between them, because unit tests have an entirely separate set of goals, techniques, and best practices which do not apply to integration tests or other sorts of tests.
Here's a quick list of some of the different kinds of tests. It's not a complete list, but it should cover a lot of common terms you'll often see tossed around at work or in the literature. As you can see, there can be some overlap between the terms: for example, you may have a developer test which is also a unit test which may also be a checkin test as well. But keep in mind they are all orthogonal concepts, and can vary independently.
- Unit test: tests a function or class in complete isolation from other code in the system. If the item under test has dependencies, those dependencies are replaced by fake ("mock") objects. If it touches the filesystem, hits a database, access a network, or displays UI, it's not a unit test. Unit tests have practically no setup and run extremely fast.
- Integration test: tests the interaction of two or more classes and functions working together. Multiple layers are usually involved. Whereas unit tests ensure that the "bricks" in your system are square, solid, and uniform, integration tests ensure that they are stacked up correctly and glued together the right way. A test that verifies data entering one end of a pipeline will come out the other end is an integration test. So is a test that verifies that data is unchanged when round-tripped through a reversible process.
- Characterization test: tests unknown or "legacy" code for response to stimuli. Often used during refactoring to ensure that the behavior of the code is unchanged. Characterization tests are often written in total ignorance of the inner workings of the code, and the expected results are often completely unknown as well.
- Developer test: any sort of test (unit test, integration test, acceptance test, stress test) written by a developer as a design and development aid, as opposed to a QA engineer ensuring the correct operation of the code after the it has been "thrown over the wall" to Test.
- Automated UI test: a test that simulates a fixed series of user interactions, such as mouse clicks and key presses, and the resulting UI is then compared to an expected result.
- Regression test: a test which ensures that bugs that get fixed stay fixed.
- Checkin test: a series of tests used to verify that the product is not accidentally broken by new features or bugfixes, prior to the change being submitted. Checkin tests do not exhaustively test everything (they still need to run in a reasonable amount of time), but they should give good coverage over certain areas. Checkin test suites are usually mostly unit tests, because unit tests execute so quickly. But integration tests and regression tests can be good candidates for inclusion as well.
*Tap* *tap* ... is this thing on?
Several years ago, when I was a wide-eyed, straight out of college, software engineering n00b, I had my first experience with a failed development project. I was too wet behind the ears to notice it at the time (and being drafted onto the project it midway into development, I wasn't totally privvy as to why or how certain decisions got made) -- but it always seemed to have a certain desperate stench about it. It seemed a bit misconceived, and halfheartedly executed besides: management never really seemed like it was ever serious about the success of the project.
Shortly after that project's inevitable demise, I found myself in a small conference room with a half-dozen of my coworkers. Our project manager was there -- well, the dev lead really; our company didn't have program managers as a separate discipline. And he was standing in front of a whiteboard, on which was scribbled the meeting's topic: "What features should be in our next project?". It was our job to brainstorm answers to that question.
Something bothered me about that meeting, but at the time I couldn't put it into words exactly what it was. It continued to stick in my mind for weeks and months. The project unfolded in due course, and as it did I began to notice those familiar symptoms again: a general feeling of aimlessness and lacksidaisical execution. Eventually the new project failed -- as did one after it, and the one after that one too. At the point where I was beginning to see a pattern, the company was collapsing around the weight of its failures, and I was mercifully laid off.
Today I'm able to articulate exactly what bugged me about that meeting, namely: there were no customers in that room. Or outside of it, for that matter. All we had was a bunch of devs pulling dumb ideas out of their asses and trying to pretend that someday, somewhere a customer might want to have this hypothetical product with all these features, even so much as to pay real money for it. We had this illusion that we were super creative geniuses and coming up with all these new ideas, but really we were just wanking. Hell, the ideas weren't all that novel anyways.
And the insidious thing was, we came up with this heaping pile of dogshit ourselves. When we complained to the project manager, he replied ... "well, it's not my fault -- it was all your ideas". And he was right. They were our ideas. We may have never believed in them, but they were our ideas.
From that day on I've always been wary of brainstorming sessions, even going so far as refusing the participate in them. There are circumstances where brainstorming sessions are just what the doctor ordered, but brainstorming as a technique is too often misused. When the problem requires down-to-earth linear reasoning, you won't find the solution with blue-sky creative dreaming.
Fact is, most projects are not hurting for the lack of ideas. Good ideas, on the other hand, are always in short supply. The trick is in vetting the ideas -- discarding the bad ones and executing on the good ones. And in that regards brainstorming is the absolute worst thing you can do. Chances are you already know what the good ideas are, but then you go out of your way to drown them in a sea of ideas of questionable merit. You may think an idea is good because it seems to be on everybody's lips, but maybe it's just a bad idea that happens to be cheap and easy to throw out there. How will you know?
Beware of the brainstorming session -- it can create the illusion of an innovative decision making process acheived by consensus -- without providing the reality thereof.