Never edit a method, always rewrite it?
dave.cheney.net
dave.cheney.net
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.
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.
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.
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.
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.
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.
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.
1. Don’t have a good understanding of the existing system architecture
2. Have not thought through the design of whatever has to be added, both in terms of function expectations or overall flow
3. Overly (?) concerned with “getting it right”; that can lead to paralysis: somehow you have to both demand the best of yourself at that moment and yet accept that despite it all you may not make the best choices - and that’s ok
- the method's signature and spec
- the changes in behavior you want
- history for file (and any bugs listed in there that had to get fixed more than once)
Looking at a diff and seeing a small modification to a method tells me more than a method that has entirely changed with for loops changed to while loops, indices replaced with iterators, different whitespace and brace placement, etc...
Obviously this does not work for side-effectful methods (beware of caches!).
In most fields we have a sample of known input vectors, including edge cases, that map to known outputs. A unit test is enough to test these methods with different implementations.
Or consider when you're performing a database migration (on large databases) with 0 downtime. Typically this involves something like:
1. Dual Writing: Create 2 tables and write to both and keep them in sync (by duplicating new data, and back-filling old data)
2. Update read paths: Change all code to read from the new table, and validate that the data being read is consistent with the old table. You can use a library like Scientist [0] to validate that the reads are the same.
3. Update write paths: Change all code to write to the new table (and raise alerts if the old path is exercised)
4. Deleting old data: Remove code and data that relies on the old data model
It kinda falls apart with refactoring but still :) I'm finding it more and more evident that file's as the default unit of storage and viewing of code is an obsolete concept
Fascinating. Could you expand on this or link somewhere?
Things like with more canonical representations of code, rather than arguing over coding standards (although it can still text-based rather than completely graphical). Ability to more easily see related code and flows. Manage the meaning of code rather than its text (not changing text but changing symbols when you refactor).
There's been attempts to move in that this direction with things like code bubbles (https://www.youtube.com/watch?v=PsPX0nElJ0k) but yet there hasn't been an approach that's both visually attractive and offers enough benefits, but it's probably coming somewhere down the line
Edit: to answer my question, yes this really exists http://cs.brown.edu/~spr/codebubbles/
[2] http://orgmode.org/worg/org-contrib/babel/intro.html#literat...
This prevents cluttering your code with old methods and also allows you to figure out if a new environment breaks your code.
Simple code scales. Write simple code.
A significant portion of functions fall in another category; functions that actually get some real work done. They are specific and not intended for reuse at all.
Applying the "never edit a method, always rewrite it"-rule to those helper/utility functions would just be painful. The public API you designed is designed with a certain purpose and reuse intent. Improving it slightly to fix an issue and forcing yourself to entirely rewrite it would just break things. Most likely you'll end up writing lots of small copies of the original method (how painful this exactly might be depends on the language you're using).
Applying that rewrite-rule to specific functions/methods makes slightly more sense. Here the logic is more tightly bound by business rules/logic, which if they change will quite often warrent a rethink. Particularly because business likes to just slap a feature on top of everything else; which means for us finding ways to keep everything sane in the codebase requires continuous refactoring. Rewriting a function here does not intend to make the function more reusable, it attempts to make the code more clear. Sometimes reusable patterns emerge, but often they don't.
Obviously the above is a simplification of things. Often programmers are obsessed with abstractions and design patterns (read too much GoF, Martin Fowler & Uncle Bob); or don't bother with anything at all (script kiddies, prototype developers, or any code written during a PhD...). The truth lies somewhere in the middle.
It's debatable if it actually helps with that intention.
A more apt metaphor might be running multiple versions of the same binary which are similar enough to coexist.
It even gets rewritten over and over during the life of a single cell.
Here we're talking about modifying a specific function, not duplicating all the code.
Programming is often a state of flow where you have a grand idea for how a mechanism should act and then you task yourself with implementing that idea in code. Sometimes large complicated functions are the best way to translate that idea. The approach the author describes sounds like it would create lots of small functions that don't really help you solve the problem at hand.
Good idea to keep in mind though. Everytime you go to edit a function and find yourself dreading it, maybe it's time to rewrite that one.
If you make this a strict requirement for your code base people write wrappers all the time, which creates a huge mess:
def newMethod(x):
if x*2 == 42:
return 36
return oldMethod(x)
You cannot enforce the requirement anyway, because people will just copy-paste the code and edit it under a new name.You just cannot push this methodology unto developers.
But, that's a sign, right? You can't ever hope to safely change that which you do not understand. If you're afraid of the codebase - I argue it's better to set aside time for making the codebase less intimidating, than to devise clever hacks/ "software process" for working around your lack of understanding. The latter will just make the codebase more intimidating, over time.
def foo_with_edge_case(x):
if x == EDGE_CASE:
return 36
return foo(x)
seems clearer to me than stuffing the edge case handling into the foo methodThough it does look nice for fib and fac.
I think it's only really doable in purely functional language (like Haskell), though. It sort of means versioning of individual functions, and also types. It's very similar to rebinding values only as opposed to modification of variables.
But naming is a problem. Maybe.. There are two kinds of names of types and functions - intrinsic and extrinsic. Intrinsic name only describes what the thing is (e.g. array.search()). Extrinsic name relates to the problem being solved (e.g. product.findByPrice()). Maybe the functions (and types) that have only extrinsic names should be versioned and short name should be assigned to the latest version.
All in all, I think it's a concept worth researching.
It creates technical debt and someone else will have to clean it up
Most people use git, but there's also cvs, svn, hg, perforce, bzr, and probably some others in use that I'm forgetting.
Leave the history to those tools, they make it easy to change things as much as you'd like without losing work.
Don't work against them with policies like this.
I guess there is a philosophical debate behind this - what's in a name? Should a name of function refer to a specific body of code only, or all possible function bodies, past and future? What did the caller of the function want?
You can only guarantee correctness if the former. But the latter gives you more flexibility. I am not saying that this is the right answer.
Keeping all versions of the same method in the source files has a lot of drawbacks:
1. Readability. Autocomplete gives me 50 versions of that method. Which one do I choose? The latest? Why do the rest need to exist?
2. Code size. Can you imagine the compilation times? IDE indexing times? Binary size?
3. Consistency / Correctness. How do you know when each call should update it's version. How do you increment all calls to the latest version?
Simplicity is the ultimate sophistication.
What would really help for that style of development would be a sort of apropos feature for your own code where you could look up methods by keywords rather than just identifiers. For larger projects, having people re-write several variations of what is basically the same method can be a problem even with mutable code. Perhaps Knuth's literate programming would help?
edit: And I bring apropos up because if you break things down into very small functions, and you don't have that, you end up writing several invariants of the same function because you can't find the one you're looking for.
One: the ability to look at code fresh, as if it were to someone wholly unfamiliar with the module or function in question. And to do so without leaping to the smarmy-tech-nerd "I understand it so this is easy" thing--we're talking about a baseline level of empathy cultivated so as to be able to return to the mindset of a novice in order to help them climb from there.
Two: the ability to explain, without the prior knowledge of that module (unless you are to reference that module, which has the same rules), what it does and why it matters. In words. Not "the code is the documentation", but words.
Three: the ability to do both of the above engagingly enough that people don't go take a nap instead of read it. You don't need to be able to write hot fire, but you need to be able to write.
These are difficult things unless you have a decent grounding in the humanities, and even on a place like HN, which self-selects for people who want to write things about stuff, you see pretty serious capability gaps on the regular.
(To be clear, I wish it was the answer, I just don't think it is. Not without a fairly radical rethinking of how much humanities matter to software development.)
Having it be compiler enforced would also be nice. Even if most of the descriptions end up along the lines of "do some stuff with the string", at least that's something.
For our Christmas campaign, calculateCostOfGadget() needs to account for the compounding discount. Preferably yesterday, as we need to complete this month's billing to make payroll.
Have functions take regular parameters and the last one will always be an associative array of options.
This matches how functions evolve. You have some required parameters, then introduce more but the old callers don't know about them so they're optional.
I was trying to argue that PHP could unify the function call syntax and array definition syntax to encourage this. In other words the options would actually be virtual, as the caller would just supply the optional named parameters at the end of the function.
Following the rewrite-not-edit method rule forces one to either sink or swim. And the only maintainable way to swim is to write small focused orthogonal methods that do one thing only. Otherwise, it is impossible at any scale to follow rewrite-not-edit.
I like take on different perspectives like this. Doing so ensures one thinks about how to design and what to code before simply diving in.
So now we come up with this technique as a way to get our fix to rewrite everything from scratch.
"Never edit a paragraph, always rewrite it".
Looks weird.
> At a recent RubyConf...
EDIT: just wanted to add that when I code in dynamically typed languages I'm inclined to do the same thing. Without rigidity provided by a strong type system it's too easy to mess things up. That's why I prefer strongly typed languages so I can benefit from the structure they enforce.