Being paid for being an asshole

"Your code sucks." I've said that more than once, some times quite literally, more recently, I tend to wrap it in a minimal bit of politeness. And much more often, I say "This [3rd party] code sucks". In fact, saying "your code sucks" and even "your design sucks" is part of my work.

YEAH! I'M PAID FOR BEING AN ASSHOLE!

Well, actually not:

Being paid for being a beancounter

At work, we write code for our embedded systems that work in industrial, medical and aerospace environments. Some of our systems are quite harmless, less dangerous than a lamp. To cause damage or to harm people, you would have to throw the systems at people. But most systems have real-time requirements, control potentially dangerous machines or oxygen supplies, or similar stuff. So errors may cause real damage, hurt or kill people. One way to reduce risks is to do peer reviews, starting way before we even think about writing code. Of course, code is also peer reviewed in nearly all of our projects. We are quite used to poke in other people's code, search weak points, and do bean counting. It improves not only our products, but also the way we write code.

Saying "your code completely sucks" is extremely rare. In fact, most times, it's the little details. Last minute changes in the code, hastily and/or interrupted, leaving a little bit of mess. Misleading names, documentation that was not updated to match changed code, left-over comments from previous iterations, a missing case in a switch, ignoring the style guide, you name it. At the end of a peer review, we have a list of problems in the code, and usually, author and reviewer agree without discussion that and how those problems have to be fixed. Sometimes, the author has to justify why and how (s)he has written a piece of code, and that this way is in fact correct. In those cases, the usual problem is lack of comments and/or documentation in the code.

Impedance mismatch

"Your code sucks" does not mean "you suck".

A while ago, we had a project that has grown too much for our little team, so we decided to subcontract a little, quite independent part of the project to an external developer. We drafted a minimal requirements list and an interface specification, added our style guide, and had a meeting with the external developer. We gave him a suitable development board, hacked to the point that relevant parts were similar to the real product, a lot of ready-to-use hardware drivers, and waited for him to come back with working code.

A second aspect of this approach was to search for someone who could help us in future projects by taking over parts of the development in busy times.

What came back was a big mess of spaghetti code, completely ignoring the style guide, lacking documentation, and hardly working at all. A classic case for "your code sucks big time", but let's face it: If you search an external developer for long-term relations, you try to be positive and helpful: "Look, we need the code to match our style guide. That's written in the contract with our customer. We need documentation, and it has at least to compile on the target CPU. Yes, your dev board has a different CPU. Use #define and #ifdef instead of hardcoding. Compile for our target, even if you can't run that on the CPU we gave you. Do this, add that, remove those, don't copy and paste, use functions, bla bla bla. This is how to use doxygen: Just add an extra asterisk at the start of the comment, bla bla bla."

Wash, rinse, repeat. The next iteration still sucked. And so did the third one. My written response to the fifth or sixth iteration was (not literally!) "your code sucks". I explained that every iteration took me more than an hour just to make it compile. I explained that we agreed on the expected behaviour of the code, but the code did not show that behaviour. That the behaviour and the form of his code were not acceptable. And that he was hired to save us time, not to cost us time.

Half an hour later, my boss came around, telling me that the external developer has cancelled the contract because of my mail. The external developer has read it as "you suck". Well ...

We agreed that my mail was not very polite, but also that the entire mail (and all previous ones) just criticized the code and documentation. My boss phoned him, and discussed more than an hour. They finally agreed on a final day in our office for handing over the code and make it run on the target system. The external developer worked with me on my computer, and we made his part of the software work on the target and added a lot of documentation.

End of the story: We had the required part of software, in a state that worked, but was still ugly. We didn't change much after that day, and so that part is still ugly. It works, and I would like to clean up the last dirty corners, but it's not worth the time. Oh, and that external developer won't be hired for new jobs.

You are too academic

In a previous job in a medical environment, I had to write an interface between an existing piece of software and a new laboratory machine that replaced an older one. The machine reported its data via RS232, and a simple external program wrote the data into a file on a file server. So I copied the old driver into a new file and tried to make sense of the existing code.

The system was written in what was originally a subset of C, but has evolved into some mix of the Hunchback of Notre-Dame, Gollum, and Salvatore from The Name of the Rose. Not quite ideal conditions for writing safe code for experienced developers, but usable. Unfortunately, the software was written by a salesman that originally just sold the IDE for Hunchback-Gollum-Salvatore (HGS - not the real name, of course). He was hired to use HGS to write that medical software, ignoring all rules for developing medical software, and bypassing the in-house IT department. He had no idea how to write software, he had no idea how to plan his time, and gave unreasonable promises of what the software would be able to do in no time. To make things even worse, a research diver was hired to help him developing the software.

I was hired to replace the salesman-developer.

So I read the driver code. It was just a single function. 1500 or 2000 lines of code in a single function. Some parts were copied five or six times instead of moving them to functions. And I found errors. Many errors. Obvious errors. Errors that no sane developer would make. Well, I was new on the job, and I was not sure if I understood all of HGS. So I RTFM, twice to be sure. I found that HGS documented that comments are "like in C". In C, comments don't nest. In the HGS compiler, comments don't nest. But in the editor of the HGS IDE, they do nest. So you end up with code that looks like it is commented out, but it is not. The compiler happily compiles it, and the runtime executes what looks like a comment in the IDE. I found that fopen returns a handle, or 0 on error. But alas, there is no way to find out what error has happened. Permissions, lack of privileges, locking, network error, non-existing file? You just get a 0 back from fopen(). There is no errno. No try-catch. No exceptions. And I found at least four more bugs in HGS itself.

Back to the driver code. There were errors about every 10 lines of code, and they were real errors, even in HGS. I looked at some of the other code, and found about the same error rate. So used a little bit of perl to simply extract all of the code that the salesman and the diver had written, and made perl count the lines of code. Then I multiplied the lines of code by the error rate from the driver code. Tens of thousands of potential errors in a medical software does not sound sane, does it?

At the next management meeting, I raised the issue. I explicitly stated that the total number of errors was a rough estimate, that may be too high by a factor of 10 if we were lucky. But even then, thousands of potential errors would remain in software were a single error could lead to severe medical complications in an emergency situation. I recommended to rewrite the software from scratch, because the existing code base was in a horrible state and HGS does not help improve the situation.

I was told by the managing director that I was "too academic". Well, if you prefer having the software help kill people ...

A merger changed a lot of priorities, and so that piece of crap was assigned to someone else, in a different federal state. I was quite happy with that decision, and even more when I heard that they had decided to outsource that project and have it reworked.

About two years later, the new old software was presented to the management, the IT and the laboratory teams. I sat in the rear corner, not really interested in that management show. "Look, new shiny buttons that look and work exactly like the old ones." But then someone from the laboratory team asked how much of the scary salesman code has survived. The presenter smiled. "We have removed almost all of that crap. It was actually easier just to start from zero than to fix the code. The software still looks and feels the same, but the errors are gone." I could not help smiling from ear to ear. The laboratory manager noticed that, raised his hand and said: "Look at Alexander, watch him smile. He told you to do exactly that two years ago."

Well, not exactly. I recommended to get rid of HGS, but the company that reworked the software was a HGS shop, so HGS stayed. I never read any line of the new code, but I'm sure that even the new code will have race conditions and will have trouble coping with I/O errors, because it is very hard to avoid race conditions in HGS, and it is nearly impossible to do sane error handling in HGS.

Alexander