What Sparked the Question
I recently leafed through The Power of TED*," a book-length parable about destructive cycles of human psychology in relationships.
At first I was annoyed by the abstractions. The "Dreaded Drama Triangle" made up of Victim, Persecutor, and Rescuer roles just seemed too neat and tidy to actually describe reality. It seemed the author was under the delusion that they'd pushed aside some curtain and glimpsed Reality Proper.
But then I realized the abstractions are just abstractions - useful heuristics to help people ask and answer the right questions. "Am I thinking of myself as a victim, and how might that impact my relationships? Do I recognize this kind of cycle in my life?"
(Aside: recognition of victim-hood can be useful. From what I can tell, the book thoughtfully avoids judgment of people who recognize "persecution" and instead focus on how to break harmful cycles of behavior. If people choose to wield the book's ideas judgmentally, that's obviously bad.)
The Question
Someone recently remarked to me, "A developer's first reaction to anyone else's code is 'This is rubbage, let's redo it all.'"
Though hyperbolic, the remark hit home. I refactor by default, and I don't have enough tools to recognize when I should leave less-than-perfect code alone.
So I wonder: How do you determine when to not refactor? What questions do you ask yourself or others? What kind of "gut feelings" do you encounter?
I'm also interested in hard-and-fast rules, but I suspect many readers will have more approximate tools at hand.
Top comments (8)
I usually run thorough this quick checklist:
General logic based on this:
This is an amazing checklist! I'm gonna save this for later!
This is great, thank you!
tl;dr ā I guess the inverse of all of this is, "When you can read through a block of code, and understand what it does on your first read-through of it, then you don't need to refactor."
Refactors are some of the most important commits you will make to an application as it grows to serve more features to more users. They serve to make code more readable and DRY, less prone to errors, more fault tolerant, and they simplify the process of building out new features by reducing the number of other sections of the app new code will need to touch.
Refactors are synonymous with revisions for clarity in prose and essays. You want the ideas to be easier to understand, but fundamentally unaltered when you're done making changes; if your changes cause the block of code to either be invoked differently, or to return a different value than before, you aren't refactoring anymore. Your test suite should let you know if your refactors have broken anything (and you obviously have a test suite, because you're a good engineer, working with a team of competent people)
To that end, the main way to tell if some code could use refactoring is the simple question, "Can I read this alright?" If you have to pause and take a moment, or go back and reread the code, to grok the actual effect of it, there's probably a refactor or two waiting to be made.
My rubric for "not readable" breaks down into a few questions about the code structure:
else if
or aswitch/case
statement?&&
/||
appear more than once in any logic branch?)1 and 2 are typically signs that you can move the nested logic of each case into methods, within classes where the conditions you're checking are inherently true. Then you can rely on polymorphic dispatch to initiate the appropriate behavior from one central method call. Instances of number 3 are typically best to move into standalone methods that return the Boolean as their result; whereas number 4 is a matter of looking at the variables that are declared, considering the system they are a part of, and figuring out what to rename them to, accordingly.
The other thing I consider when asking if I should refactor some code, is whether or not doing so will enable future development to go more rapidly. More often than not, the answer will be "yes", but it's always possible to decide to refactor too early, and lock yourself in to a more complicated abstraction.
I hope that helps!
First off, wow, the depth here is fantastic. Thanks so much for taking the time.
I'm struggling to follow your recommended refactor for #1 and #2. Maybe I'm hung on the term "polymorphic dispatch" (no matter how many times I read descriptions of polymorphism, the meaning doesn't stick to the word for me).
Here's an example of what I think you mean by #2:
If that is an accurate example, how would you refactor with polymorphic dispatch?
Again, thanks so much. I'm honestly daunted trying to learn when to avoid refactoring, and this is super helpful.
I'm glad I could help! Refactors used to be my least favorite aspect of writing code, but once a mentor demonstrated how important they are to having maintainable, updateable code, keeping my own codebases tidy became a matter of respect to him, and pride to myself š
To your question: Yes, that's exactly the sort of scenario I was thinking of! I haven't used TypeScript before, so someone else will have to let you know if this compiles, but I essentially mean that you would give two methods on different classes the same signature, like so:
That will allow you to remove the entire
if-else if-else
tree that pertains to handling the check against the passed-intype
Now, no matter how many types of contact request you submit through this function, adding support for a new type is an O(1) developer problem, because you can guarantee that adding e.g.
FaxRequest#handleSubmit
will not ever interfere with the behavior of eitherPhoneRequest
orEmailRequest
's own#handleSubmit
method.Let me know if you have any other questions!
No worries about the syntax. I kinda invented a C#-TypeScript hybrid there. :-)
That's really helpful! I like your description of developer-complexity as O(1). I'll probably return to this example when looking at future refactors. Thanks again!
when you look into a piece of code and don't think about the word 'refactor'.