6 ms·
It has been my experience that after a substantial period of being in real use, methods embody tacit knowledge about the problem domain that isn't necessarily e
by JulianMorrison 9y ago
It has been my experience that after a substantial period of being in real use, methods embody tacit knowledge about the problem domain that isn't necessarily evident from their public interface. Because initially they didn't embody that, and it caused bugs, so people edited in fixes.
It has been my experience that rewrites from scratch typically lose all this tacit knowledge and re-implement the original bugs.
- DavidWoof 9y agoThat's sort of the point of the suggestion, I think. If you always rewrite from scratch rather than edit, this sort of tacit knowledge won't get embedded undocumented and untested into the middle of a function. 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.
- kibwen 9y agoOne thing the OP doesn't address is, is the rewrite intended to be black-box or not? If not, then every "rewrite" will just be someone copy-pasting the old code with the new fixes, which seems completely pointless. And if it is a black-box rewrite, then all that tacit knowledge will be lost, leading to having the exact same bugs over and over and over and over again, forever. 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.
- DavidWoof 9y ago> And if it is a black-box rewrite, then all that tacit knowledge will be lost, 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.
- aidenn0 9y agoI'm glad it's immediately farcical because it should help people from not interpreting it as actual advice (even with as farcical as it is, I see many comments here that miss that point).
- tvst 9y agoI'm not convinced of the "always rewrite" thing, but what you're talking about would be solved by writing tests when you fix a bug.
- koonsolo 9y agoIt's not the first time someone proposes to write a test when fixing a bug, but you have to question the actual ROI. 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.
- Klathmon 9y agoWe use the test as a way to prove we know what the bug actually is. 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.
- hacker_9 9y agoWhist I agree with this approach, sometimes it just isn't possible if the code was written in a non-testable way to begin with. Such as code that hits the database a lot or relies on network data. In this case, weighing up the ROI is a worthwhile endeavour, with a note to refactor more thoroughly at a later date.
- acjohnson55 9y agoThat's a really good reason to write business logic in a way that doesn't have those dependencies hard wired in. I often end up in those sorts of nightmares in codebases that use the active record pattern (hiding database access everywhere), hooks, critical logic in the HTTP layer, and so on.
- soVeryTired 9y agoThe 'Chesterton's Fence' school of engineering... https://en.wikipedia.org/wiki/Wikipedia:Chesterton%27s_fence https://en.wikipedia.org/wiki/Wikipedia:Chesterton%27s_fence
- cptskippy 9y agoYou're not wrong. 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.
- mcherm 9y agoUnit tests can only test certain kinds of behavior of the function under test. To give other kinds of examples, the current implementation might have used a clever trick to achieve faster execution (this cannot be evaluated with unit tests). Alternately, it might have been implemented so as to share common functionality with a different piece of code by calling a common subroutine. That sharing of functionality is probably not easy to evaluate with a unit test (nor is it wise), but is still an important property to preserve.
- 0xcde4c3db 9y agoThe most "fun" subcategory of "how do I write a test for this?" that I've seen is when something in your software triggers a bug in somebody else's product, and "somebody else" is both too big and important to return your calls and too relied-upon for the bug to go unaddressed (e.g. the Oracles and Ciscos of the world). So you have little recourse beyond screwing around with your code until the user stops seeing the bug.
- cptskippy 9y agoThat happens all the time. I generally encapsulate the logic compensating for the bug and decorate it with copious comments explaining the rational then write a unit test explicitly testing for the buggy behavior. If the bug gets fixed then the test fails and the product doesn't ship until it's resolved. 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.
- psyc 9y agoI’ve seen this in two broad circumstances: complex business rules, and spaghetti code. Business rules mostly have to be endured. Spaghetti code should be untangled if the software is still actively developed. This caveat appears much less often in editors, design applications, general horizontal apps and games. Those are more likely to suffer from poor design or coding, and the spec is much more in the control of the software people.
- dionidium 9y agoThis is the central argument in this famous piece by Joel Spolsky: https://www.joelonsoftware.com/2000/04/06/things-you-should-never-do-part-i/ 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."
- dkarl 9y agoOften people suggest rewriting a method because it's so complex and poorly understood that it can't be maintained as-is, and for a method like that, you're right, it probably can't be rewritten without inadvertently changing the behavior. 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.
- Reefersleep 9y agoDo two pull requests: one pure refactor, one pure logical change. I've been doing this at work, and it's great!
- onlyrealcuzzo 9y agoHey Julian, I have a startup that attempts to address this in a developer friendly way. I'd love to get your feedback if you'd like to see it. Reach out to me at yahn007 @ Gmail dot com. Hope to chat soon!