00:00:30.480
Every once in a while, I come across a bug that I can't help but admire. I have trouble sleeping, and I forget to eat. It's an adventure, bugs like these consume me; it's exciting and stimulating. When I figure out the devious ways in which they're causing the system to fail, I feel victorious—Princess one, Dragon zero.
00:00:59.280
This particular bug was the other kind of bug—the mind-numbingly boring kind. The kind that causes all the other items on my to-do list to seem suddenly important. Surely I can't fix this bug right now; I have a toilet to scrub. The bug was in an application whose sole purpose was to allow villagers—actual flesh-and-blood villagers—to post gossip on the web. So, who got hitched? Who had a baby? Who has a birthday coming up? The gossip would then get piped straight into the production system of the local village rag where it would be printed on actual paper, alongside the story of the largest fish to be caught off the dock since 1962.
00:01:45.599
The broken feature was the one that, when Alice posts a picture of her new baby, allows Bob to tell everyone that they should come gossip about it. In other words, the broken feature was being able to gossip about the gossip. The strategy I chose to solve this problem was to get the feature fixed as quickly as I could, preferably without reading any code.
00:02:06.479
The full bug report read as follows: 'I tried to share by email, but nothing happened. I opened up the application, put in my email address, clicked the button, and lo and behold, the villager was right—nothing happened. I activated the JavaScript console and tried again, only to get a 500 error. The JavaScript was trying to make an asynchronous call to an endpoint inside the application, and the response that came back in the little debugging window was a wall of text formatted as HTML. So, I copied out the call with all the parameters so I could call the endpoint using curl, redirect the output into a file, and finally open it in a browser.' The stack trace was exactly one line long.
00:02:49.680
The execution hadn't even reached the controller action; some before filter blew up trying to send a message to nil. A quick investigation revealed that the before filter was completely irrelevant to encouraging gossip, and the solution was to bypass it altogether. Now, I was expecting to get a 200 OK, a snippet of JSON, and an email in my inbox. Instead, I got another stack trace formatted as HTML. This time, the code managed to get all the way to the email template, the ERB template, before it blew up due to something related to translations, and the stack trace drew out the entire feature.
00:03:22.239
It starts in the controller, in a method with the outrageous name 'share by email or SMS.' This calls out to a module called sharing, which has a method called 'send to recipients,' presumably a collective term for email addresses and phone numbers. The next stop in the stack trace is 'send to email' in the same module. This further delegates to an action mailer that sends an email using a text template, which is internationalized and interprets strings from a locale file. The error message actually displayed the exact line of code that was failing.
00:04:02.000
The internationalization thing is some fancy string interpolation that takes four arguments, and the ERB template was sending in only three of them. By matching up the keys in the error message, it was pretty straightforward to determine that the missing key was 'tidbit sender.' So, I patched the template, kicked off the email, and at this point, I did get back a proper response, and the email arrived in my inbox. My work here was done. Management would be impressed—a heroic save and all that.
00:06:04.800
The truth is, I did what the French call the 'minimum Sandy Cal.' This is a term that mocks the bare minimum; it makes fun of bureaucracy for setting standards that are so low that you literally could not do worse because at that point, you would not have done it at all. Now, having fixed this feature, I was sorely tempted to tiptoe away. I knew that if more bugs were discovered a few months down the line, I wouldn't be accused of negligence. My reputation would be pristine; my name is not even in the commit history for the feature aside from those two lines. So I’d be clear.
00:07:30.480
Still, I was unable to turn my back on this code. I'd love to pretend that the reason I couldn't leave this alone is that I'm a good person, a morally superior being, but I'd be lying. I'm driven by fear. The code might be decent or it could be a stinking quagmire of rot and decay, and without at least glancing at it, I can't know. I fear stinking quagmires of rot and decay; I dread getting sucked into them and being thigh-high in mud and leeches, wrangling boolean gates with one hand and fighting off stack traces with the other. The last thing I want to do is wade into that swamp.
00:08:18.319
But I fear that it’s going to cause me x hours of pain—let's say three. If this blows up in my face at 2:45 AM one night, I am likely to spend those three hours getting it wrong, causing me three to four days of mopping up hell and pain, writing scripts to parse access logs so that I can put lost orders back in the database—hypothetically. It's this fear that motivated me to take one tiny step to eyeball the code, just look at it and make an evaluation about how likely it is to blow up in my face at a particularly inconvenient point in time.
00:08:59.680
So before we look at the code, I'd like to take a moment to talk about the tests. Right, moving on.
00:09:06.000
In the controller, in the 'share by email or SMS' action, the first thing that caught my eye was a constant that I had never seen before—tiny URL. It was not present in the stack trace; the email I received did not contain a shortened URL. The logical explanation is that this is only used for the SMS portion of the feature, and it's true that we are building up the text message for the SMS here in the controller on every request, whether or not we've received any phone numbers to send the SMS to. However, the text message also does not contain a shortened URL.
00:10:03.360
In fact, once the URL tinyurl is generated and assigned to this local variable, that variable is never referenced again. So let me show you the tinyurl module. Actually, let's have the comments in the code tell us a little bit about it. We have a base exception class. Incidentally, the only thing the base exception class is used for is to define this exception class, which in turn gets used exactly once.
00:10:42.000
This is a bit cryptic, but the method name does a pretty good job of explaining it. This is both cryptic and misspelled, but again, the method name explains what's going on. So enough about comments; let's look at some code. This is the core piece of tinyurl. Now, you don't have to read the code; just notice that we're calling out to an external service over HTTP every time we call the 'share by email or send SMS' action.
00:11:19.280
Now, the author of this code has thoughtfully added caching, so we might not make the call on every single request. Besides, it only happens in staging and production. You'll notice that the case statement always returns, and therefore the last nine lines will never be reached. Also, you can see that the case statement has exactly two outcomes: true or false. We do have simpler means of achieving the same result. The caching mechanism manages to use both a module variable and a module instance variable in the same breath.
00:12:01.360
It turns out that the tinyurl module is not used by any code anywhere—ever—so it can safely be deleted. The code has been racking up expletive points pretty quickly now. Expletive points are, well, expletive points if you're American; they're also known as XP. They're an equivalent measure to 'wtfs per minute.' So, we found two bugs where only one was expected. Bugs congregate—there's dead code, there's a gratuitous case statement, the code uses a module variable, the code uses a module instance variable.
00:12:47.440
The comments either state the obvious or are incoherent. We're building a text message even when we're only sending emails. The entry point into the feature is a method called 'share by email or SMS.' Apparently, the method does two things that are so unrelated we cannot even use them in the method name. There are no tests. The code is calling out over HTTP on every request and never using the result.
00:13:24.000
We've accumulated 62 expletive points. But wait, there's more. An 'unless' is being used to determine the response for the action, and it's switching on a flag called 'invalid name,' giving us a double negative for the happy path. The invalid name flag is actually pretty spectacular; a name must not exceed 40 characters to be valid. The significance of the number 40 is unknown. Out of curiosity, I kicked off the curl command with an invalid name, and I received a reasonable error message and an email.
00:13:55.760
So the process goes like this: having determined that the name is invalid, we send out the emails and text messages and then return an HTTP 400 Bad Request. You'll notice that nil is totally legit now. It seems likely that if we hit the controller action without a name, it would result in an email being sent out. Instead, we get a 400 Bad Request, and the error message tells us that they have already been told, obviously.
00:14:30.000
Now it turns out we don't get an email because the 'send to recipients' method has a guard clause against nil names. Let's look at what else it has to offer. In the sharing module, the 'send to recipients' method is complicated looking, and it has a comment that explains what the method does. Unfortunately, every single part of that comment is wrong.
00:14:55.280
There is no 'message' key; the hash key is called 'sms.' There is no 'email recipients' key; it's just 'email,' and there's no 'sms recipients' key; it's called 'numbers.' You'd think that it can't be more wrong, but it is. Even if all the keys were named correctly in the comment, the comment is still wrong because the message that gets built up and sent via params doesn't actually get sent to emails. We have an ERB template that the action mailer uses instead.
00:15:41.280
The lesson is that comments that restate the method's implementation are bound to decay and should be deleted. So the method has code to clean up the text message, but that code never gets called because the params hash never has a message key in it. Then, it cleans up the name. This is reasonable aside from the fact that the controller validated the length of the name to be less than 40 characters before passing it to this method. Now, having cleaned up the name, it could now be shorter and therefore valid.
00:16:15.760
Then it goes ahead and validates the length of the name again, this time with a cutoff of 41. So once we've gotten back past all of the validation and the cleanup, we get to this block. Let's break this down: we combine emails and phone numbers that came in separately into a single array. Then, we iterate over the whole lot and use regexes to figure out whether we have a phone number or an email so that we can dispatch the notification using the correct protocol.
00:17:23.120
At the very core of this block, we have two methods: 'send to email' and 'send to SMS.' Let's look at 'send to email' first. According to the comment, the 'send to email' method sends email, but the comment is only partially correct. This is very confusing, so I'll go through it step by step: Alice posts a picture of her new baby. Bob goes to the website and enters Charlie's email address to tell him about it. The tidbit was created by Alice, and if we don't have her email address, we will skip blithely past the part where we actually dispatch the email to Charlie, whose address we have because Bob gave it to us.
00:18:24.000
Here's the thing: in this application, it's very likely that we don't have Alice's email address because she signed up using her cell phone. If she forgets her password, we're going to send her an SMS, not an email. We prefer to report good news always. So remember this block of code; I actually simplified it. Originally, it looked a lot more like this: returning true here ultimately causes us to tell Bob that Charlie got the email, regardless of what actually happened. In the other branch of the conditional, we send the 'send to SMS' message, which, according to the comment, sends SMS.
00:19:15.840
There are lies, damn lies, and comments. So what does this code say about the developer who wrote it? Well, it's easy to criticize and a whole lot of fun. It's tempting to believe that whoever wrote the feature is stupid or a bad developer, or at the very least a bad person. This is known as the fundamental attribution error. When somebody writes terrible code, we make assumptions about why that is. On one end of the scale, we attribute bad code to individual traits—their personal fixed characteristics—they're not skilled enough, or they don't care enough.
00:19:48.640
At the other end of the scale, it's situational—a momentary lapse due to contextual pressures. So on one end of the scale, they're a bad person; on the other end of the scale, they're having a bad day. The forces that push us around from moment to moment are invisible. We notice them; we're in the thick of it, in our own lives, and we can't help but notice them. But from the outside, these pressures are hard to see, and the farther we are from the situation, the harder it is to imagine them. As a result, people we don't know are bad developers, whereas we and our friends are just having a bad day.
00:20:30.720
So, what happened with 'share by email or SMS'? How does a smart, experienced developer like the author of this code end up with a mess like that? Surely they didn't sit down one morning, write the feature from one end to the other, and finish with a flourish. Most likely, what happened to them is what happens to all of us: rush jobs, rapidly changing requirements, corners that get cut. Even without pressure, this was not an ideal situation. For one, this developer wasn't actually on the project. This gossip application—that was me and one other guy—resulted in an unfamiliar code base for them, and second, the code base was terrible.
00:21:25.680
I mean, it was not exemplary code. It's not a code base you'd like to inherit. Then again, who knows? Maybe there was no deadline, maybe the programmer understood the code perfectly, and maybe they were just born or drunk. If you strip away all the fuzzy, muddled, messy humanity of it all, you get down to an interesting equation based purely on rational choices.
00:22:24.160
So imagine a scenario where Player One and Player Two may either cooperate or defect. If both players choose to cooperate, they both get three points; this is a reward for mutual cooperation. If both players choose to defect, they get one point each—a punishment for mutual defection. If Player One cooperates and Player Two defects, then Player Two gets five points, known as the temptation to defect, while Player One gets zero points, known as the sucker's payoff. In terms of software, cooperating means coding with care—whatever that means to you or your team.
00:23:17.520
It could be taking the time to add tests, making sure that your methods are short, that their names are expressive, that you've deleted all the trailing white space. Defecting means not doing those things—committing with no thought to how much sense this will make to the next person who's going to see it. So here's the thing: if Player One knows that Player Two is going to defect, then it is in Player One's best interest to defect as well. Both players get one point.
00:23:47.840
On the other hand, if Player One knows that Player Two is going to cooperate, then it is in their best interest to defect, and they're going to get five points while Player Two will get none. The rational choice here is always to defect, and yet, by doing so, everybody loses. Notice that this is not a zero-sum game. The players do not have strictly opposing interests. The largest number of points on the table for a single round is six, and the only way you can get that is if the players cooperate.
00:24:33.840
Defection is only the rational choice in a single-shot game or a situation where the number of games is fixed—like when the time period is fixed. There's no incentive to cooperate if you're never going to play against the same player again. The dynamics change completely if Player One and Player Two interact on an ongoing basis. Now, in most realistic situations, you don't know when the last interaction will take place. So if Player One and Player Two play an indefinite number of successive rounds in a game, then when one player defects, the other player has the option of retaliating in response in the next round.
00:25:15.520
In this situation, there is no single strategy that will always do well. If the other player chooses a strategy of always defecting, the best you can do is to always defect as well. So now what if the other player chooses the strategy of permanent retaliation: they start off by cooperating, and then you defect once, and they never cooperate again. In this situation, the best you can do is to always cooperate.
00:26:30.160
So what's a rational person to do? It turns out that the most robust strategy across the largest number of situations is as follows: cooperate at first until the other player defects, at which point you retaliate immediately. If they back down when you retaliate, forgive them completely, hold no grudges, and start cooperating again as quickly as possible. This is game theory 101 suggesting that you should be nice. Now, they have a very particular definition of nice. It doesn't involve being a pushover.
00:27:21.600
In game theory, nice means don't be the first to defect. If the other player never defects, it's all good—everyone wins. There's a great book that covers a lot of the research done in the late 70s and early 80s called 'The Evolution of Cooperation,' which discusses computer programs pitted against each other in a grand tournament. It also highlights the follow-up research done after the tournament, looking at the fundamental problem of cooperation in society, starting with the question: when should you cooperate and when should you defect in an ongoing relationship with another person?
00:28:37.360
The results of the tournament were so surprising that researchers began to look at what makes it possible for cooperation to evolve in an environment of pure self-interest. The book transforms into a fascinating exploration of evolution from a strategic, not genetic, standpoint. The strategy that won both rounds of the tournament was called 'tit for tat.' It was the shortest program submitted, and it worked like this: cooperate on the first move and then do whatever the other program did in the previous move. That was it.
00:29:04.200
The thing that made it so successful was that it avoided unnecessary conflict. It was highly provocable, and it forgave immediately. Also, it was incredibly easy to understand, so the other program could adapt accordingly. So, I am making software out to be some grand competition where developers compete against each other, which is ludicrous. The problem is that we often behave as though it's the case.
00:29:07.600
This abstract formulation maps shockingly well to the costs we pay to deliver software. When everyone on the team cooperates, the team delivers good software with a reliable cadence. Everyone looks good; everyone can be productive. If one person on the team defects, then that person looks efficient, taking off features from the backlog. They're fixing bug after bug after bug, many of which they introduced in a previous fix. They're productive; they deliver all sorts of heroic saves quickly and reliably, and the rest of the team slows down trying to understand the code this person is writing.
00:29:59.680
We'll have to rework things whenever and wherever changes are made, and this adds time to every change. If the whole team defects, there's an initial rush where features are delivered rapidly, but soon we hit a point where each change takes longer and longer to implement until we reach a pathetic but stable equilibrium where progress inches forward in an endless, painful struggle. Humans are biased towards cooperation; most of the time we'll cooperate, even when it makes no rational sense. Yet, in being irrational, we will also gleefully defect, even when we have no incentive to do so.
00:31:03.600
Sometimes, it's just easier to not— you know, to not write those tests, not rename those variables. We want to do the right thing, but how often do we slip? How often are we in a rush for no particular reason other than habit? How often are we tired, flustered, or distracted? How often do we commit and just move on with our day? Good intentions are meaningless. Every commit we make tips the balance ever so slightly in one direction or the other—an inch towards entropy or an inch towards increased order.
00:32:15.360
Perfection is unattainable and ultimately irrelevant. If most of the commits you make are shifting the balance towards increased order, no matter how slight that shift is, you will have a code base that constantly improves. So I urge you to ask yourself before you commit: am I cooperating or am I defecting? Thank you.
00:29:03.120
You.