Re (2): This is also a great example of the perils of writing a complex algorithm and failing to provide overview documentation (like this article!), not just inline comments.
However, in the absence of an overview, if the code that you don't understand was written by jwz, you might want to study it very hard before removing it. :)
If the code works you'd damn well better understand it completely before rewriting
But then, one of the primary reasons that usually motivates a rewrite is that nobody understands the old code. (sometimes acknowledged as such, and other times indirectly in the form of "every time we try to fix a bug we break something else, this code is terrible").
Re-writing code that nobody understands and hoping that it will work correctly is just wishful thinking. If people really wanted to understand the code, they could do the hard work of reading it, tracing it, writing tests for it, etc. In many cases, you can transform the code into something readable by applying a long series of simple refactorings.
For some code, breaking behavior you don't understand doesn't make a difference. If you're Facebook, you can arbitrarily change the user interface of your site and your users have no say in the matter, since they're not paying customers. But other developers don't have it that easy. If you support code that people have built their own applications on top of, you can't just break stuff. If you make backward incompatible changes to the Linux kernel APIs, or break Microsoft Excel so that macros that have been working for years stop working, people all over the world will be very unhappy.
If you have a well defined spec and a reasonable test suite, you can throw away the code nobody understands and still have the replacement code work correctly.
The chances of having a well defined spec and reasonable test suite in a place where there's code nobody understands are left as a calculation exercise for the reader (but I'd start at 5% and work downwards.)
It's not likely you'll ever find an accurate spec for a piece of legacy software that's been around for years. Even if there was a spec that completely defined the original behavior (which is doubtful), that spec probably won't reflect all the new features and other changes that were added over the years. You'd have to merge the original product spec with all the new feature and change specs and hope that you didn't miss anything. (In many cases, it takes a bug report to realize that an item in the spec was defined incorrectly, incompletely or ambiguously.)
In practice, I think that the only "spec" that's likely to capture the exact behavior of the code as it exists today is the code itself.
Also, the lack of a complete spec probably implies a lack of a complete acceptance test suite. Note that I said "acceptance test", not "unit test". Unit tests from the original code are useless for testing the re-written code, since they're specific to the particular implementation of the product you already have, which may have a completely different set of classes from the one you'll be replacing it with.
The way this should have been done is there should have been a unit test that tested the correctness of whatever the
"dummy thread parent" was supposed to achieve. With such a test in hand the new implementation would have been obviously deficient.
Of course, in accordance with another of jwz's laws, the CADT model of software development, test suites are often discarded along with everything else when some attention-deficit teenager decides to rewrite everything from scratch.
Comments
Re (2): This is also a great example of the perils of writing a complex algorithm and failing to provide overview documentation (like this article!), not just inline comments.
However, in the absence of an overview, if the code that you don't understand was written by jwz, you might want to study it very hard before removing it. :)
Doesn't matter who wrote it. If the code works you'd damn well better understand it completely before rewriting.
But then, one of the primary reasons that usually motivates a rewrite is that nobody understands the old code. (sometimes acknowledged as such, and other times indirectly in the form of "every time we try to fix a bug we break something else, this code is terrible").
Re-writing code that nobody understands and hoping that it will work correctly is just wishful thinking. If people really wanted to understand the code, they could do the hard work of reading it, tracing it, writing tests for it, etc. In many cases, you can transform the code into something readable by applying a long series of simple refactorings.
For some code, breaking behavior you don't understand doesn't make a difference. If you're Facebook, you can arbitrarily change the user interface of your site and your users have no say in the matter, since they're not paying customers. But other developers don't have it that easy. If you support code that people have built their own applications on top of, you can't just break stuff. If you make backward incompatible changes to the Linux kernel APIs, or break Microsoft Excel so that macros that have been working for years stop working, people all over the world will be very unhappy.
If you have a well defined spec and a reasonable test suite, you can throw away the code nobody understands and still have the replacement code work correctly.
The chances of having a well defined spec and reasonable test suite in a place where there's code nobody understands are left as a calculation exercise for the reader (but I'd start at 5% and work downwards.)
It's not likely you'll ever find an accurate spec for a piece of legacy software that's been around for years. Even if there was a spec that completely defined the original behavior (which is doubtful), that spec probably won't reflect all the new features and other changes that were added over the years. You'd have to merge the original product spec with all the new feature and change specs and hope that you didn't miss anything. (In many cases, it takes a bug report to realize that an item in the spec was defined incorrectly, incompletely or ambiguously.)
In practice, I think that the only "spec" that's likely to capture the exact behavior of the code as it exists today is the code itself.
Also, the lack of a complete spec probably implies a lack of a complete acceptance test suite. Note that I said "acceptance test", not "unit test". Unit tests from the original code are useless for testing the re-written code, since they're specific to the particular implementation of the product you already have, which may have a completely different set of classes from the one you'll be replacing it with.
Indeed and I covered all that in my second paragraph.
The way this should have been done is there should have been a unit test that tested the correctness of whatever the "dummy thread parent" was supposed to achieve. With such a test in hand the new implementation would have been obviously deficient.
Of course, in accordance with another of jwz's laws, the CADT model of software development, test suites are often discarded along with everything else when some attention-deficit teenager decides to rewrite everything from scratch.
It's not like jwz was dead or trapped incommunicado in the arctic. Did no one bother to just ask him?