It has been my experience that rewrites from scratch typically lose all this tacit knowledge and re-implement the original bugs.
This is sort of the great contradiction of software development. Untested legacy software is hard to work with. So should our focus be on how to work with legacy software or how not to create it in the first place? There's really no answer to that.
I understand that this is all just a thought experiment, but there has to be a better way of framing it that isn't so immediately farcical.
This is a thought experiment on how to avoid that tacit knowledge, not how to deal with it. Consider approaching every edit in the same way you'd approach new code: is the function adequately tested, does the function have a single responsibility, is the intent clear, etc.? The idea is that if you do this, then you won't reach the point where you have a 200-line method with tons of hidden business rules.
I agree there's probably a better way of framing this. Still, as a thought experiment, it's worth thinking about when you can and/or should do this.
In theory if every time a fix was implemented, a corresponding unit test was created then you should be able to adequately test the rewritten method.
I say in theory though because that never happens.
Yes I write unit tests for 3rd party APIs and libraries I consume if they show themselves to be inconsistent or buggy.
We actually use a 3rd party API that crashes regularly and we have a suite of unit tests surrounding it so when reports come in we can isolate, reproduce, and verify the problem before bringing fury down upon the vendor.
It is also handy during pre and post deployment validation to ensure that the environment your update is working in is 100% functional. I've rolled back releases before because an external resource had an unreported outage that caused our deployment validation to fail. With pre/post test you can test the environment before you even start.
Sure it can. I worked on a project with a pubic API with many time sensitive routines and we most definitely had unit tests measuring performance. The test failed if the execution time exceeded the acceptable threshold.
> Alternately, it might have been implemented so as to share common functionality with a different piece of code by calling a common subroutine.
You can certainly write coherency tests to ensure that two methods are doing what's expected. They might not share the same subroutine any longer but you can throw an exception when their respective results no longer align and have comments on the test explaining the rationale. Honestly if you have this sort of subtle dependency and you don't have a unit test for it then you're just asking for trouble.
In order to not be beholden to specific hardware/execution environment and absolute times, you can also compare an unoptimized version against an optimized version, so the test can be: optimized version must be 2x faster than unoptimized.
Of course if you actually have real-time requirements, it is good to test against those.
https://www.joelonsoftware.com/2000/04/06/things-you-should-...
To wit:
"Back to that two page function. Yes, I know, it’s just a simple function to display a window, but it has grown little hairs and stuff on it and nobody knows why. Well, I’ll tell you why: those are bug fixes. One of them fixes that bug that Nancy had when she tried to install the thing on a computer that didn’t have Internet Explorer. Another one fixes that bug that occurs in low memory conditions. Another one fixes that bug that occurred when the file is on a floppy disk and the user yanks out the disk in the middle. That LoadLibrary call is ugly but it makes the code work on old versions of Windows 95."
At my previous job, they checked how likely it is for a fixed bug to reappear again. Odds are really low. So we decided it wasn't worth the investment to write a test specific for the bugs use-case, because it's unlikely to appear again. We focused our efforts to other, more effective things.
Our debugging "path" normally is:
1. Find out bug exists
2. Write a test that can reproduce the bug
3. Distill that test down to the minimum needed
4. Fix bug until test passes
5. Test bug is actually fixed by trying to reproduce it the same way the user/bugreport did.
When done like that, the test isn't "extra work" but is just part of narrowing down your actual problem most of the time.
And that test (assuming it's not extremely slow to run), takes very little to maintain, so leaving it in the suite of tests is basically a positive only.
well the first problem is not a problem since you can cheat it. if you rely on the native database you can actually fake your driver and insert transactions (subtransactions or in postgresql savepoints) so for the whole test suite you just rollback to the latest savepoint, this saves a lot of time. (it's actually not that hard in java+di or in languages where you can monkey patc code.
What I see in refactoring is that whole blocks of code get an overhaul. And in that case, all the test you wrote on those interfaces need to be adapted, rewritten or removed.
So actually, in a proper refactoring, some tests might help you out, because the interface of the classes didn't change. But other tests cost you time.
It's all a pretty complex balancing act. Some tests save you time, some cost you time. They all make the code more stable, but you have to put your effort where it really makes sense.
I find the ROI for tests is high despite the large number of worthless tests because of the ones that fail a small number alert me to something I wouldn't have found otherwise.
The 80% number was of course made up. In my experience it is realistic, but I have never done a formal study.
https://en.wikipedia.org/wiki/Wikipedia:Chesterton%27s_fence
I think what is being proposed here is rewriting methods that people usually don't consider because they're basically okay. Maybe the name is a little weird now because the usage of the method has changed. Maybe two methods that used to do something different now do effectively the same thing. But everything is basically correct and readable, and people don't want to make a bigger change than necessary.
Personally, I think I lean towards more aggressive rewriting than other people do. I try to make sure names make sense. When special cases are removed I check to see if this allows me to make the rest of the code simpler. When I see the same thing being accomplished in different ways in the same file, I'll take a minute to make it consistent, so that same things look the same.
But people see this as a trade-off, especially when it comes to code review. Several times when I've made major changes that needed to be code reviewed, I've taken extra time to re-order my commits so the change can be reviewed in two steps, first reviewing the refactoring that was done and then reviewing the change that was made. And with a single exception nobody has taken me up on it; they've insisted on reviewing the entire change at once. Instead of a straightforward refactoring and a straightforward change in functionality, now they're trying to make sense of a combination of behavior-preserving and behavior-altering changes. Because code gets reviewed that way, people tend to refactor less than is optimal, because they want to give their coworkers small diffs that highlight the logical change that was made. This is a tendency that can lead to a gradual accumulation of history in the form of duplicated code and nonsensical names, which I think is what the "rewrite every time" rule is meant to counteract. A technical rule won't make people forget a social trade-off, but it might help them remember the other side of the trade-off.