[HN Gopher] Reasons Not to Refactor
___________________________________________________________________
Reasons Not to Refactor
Author : chmaynard
Score : 68 points
Date : 2025-02-04 04:57 UTC (2 days ago)
(HTM) web link (thoughtbot.com)
(TXT) w3m dump (thoughtbot.com)
| darioush wrote:
| another reason to not refactor is management will start out
| supportive of it but some weeks in they will wonder wtf their
| engineers are doing all day and why aren't they building the new
| pile of stuff they want built yesterday.
| eyelidlessness wrote:
| That's not so much a _reason_ , in my experience, as a frequent
| default/inertia position. In other words, I expect to start at
| that default, and challenge it _with reasoning_ , in order to
| justify a significant refactor.
| svachalek wrote:
| Refactoring is a terrible project plan. You never want your
| weekly status to be "refactored some more". Plan your other
| development work with enough time to refactor as you go.
| lesuorac wrote:
| The ability to ship small things is I think something really
| missed in discussion about microservices.
|
| Like rather then refactoring the entire application to support
| a faster cache or something you can just add in another
| microservice that handles caching of the data.
|
| If some way you write the services is poor you can rewrite one
| of the services a new way and easily evaluate if it's actually
| a good idea to change the rest or not.
| IshKebab wrote:
| Don't spend 100% of your time refactoring.
| gavmor wrote:
| Well, yeah, that example decreases symmetry.[0]
|
| 0. https://bensguide.substack.com/i/144306439/local-symmetries
| lelanthran wrote:
| Simple and brief rules are more successful in practice than long
| and complicated rules.
|
| I feel a briefer and more-to-the-point _" When To Refactor"_
| guide is to ask the following questions _in the following order_
| and only proceed when you can answer _YES_ to every single
| question.
|
| 1. Do we have test coverage of the use-cases that are affected?
|
| 2. Are any non-trivial logic and business changes on the horizon
| for the code in question?
|
| 3. Has the code in question been undergoing multiple
| modifications in the last two/three/four weeks/months/years?
|
| Honestly, if you answer _NO_ to any of the questions above, you
| 're in for a world of hurt and expense if you then proceed to
| refactor.
|
| That last one might seem a bit of a reach, but the reality is
| that if there is some code in production that has been working
| unchanged for the last two years, you're wasting your time
| refactoring it.
|
| More importantly, no changes over the last few years means that
| absolutely no one in the company has in-depth and current
| knowledge of how that code works, so a refactor is pointless
| because no one knows what the specific problems actually are.
| andrei_says_ wrote:
| These are very simple and clear and confirm my own approach to
| refactoring. Thank you.
| eyelidlessness wrote:
| One reason I might accept one or more NOs to your questions:
|
| Does the refactor support pending work, which isn't directly
| related to the refactored code, but benefits from the lessons
| learned and applied in the refactor... even in some indirect
| way?
|
| This might be providing a clearer pattern you'll apply to
| similar new functionality; or it might be providing a new
| abstraction _or even eliminating a failed abstraction_ which
| sets that pending work on the right path.
| AnimalMuppet wrote:
| Yeah. I did a (very small) refactor. It took, IIRC, four
| days. When I was done, I could write the new thing I was
| implementing in 10 new lines that used the newly-refactored
| existing code.
| PaulHoule wrote:
| I'd argue with that. Small-scale "micro-refactoring" operations
| are safe almost by definition. [1]
|
| It depends on your language. In a large Python system [2], all
| bets are off, but in Java if you use your IDE to rename a
| method to have a clearer name or change the signature of a
| method or extract or inline a method the risk of breaking
| anything is close to zero whether or not you have tests.
|
| Personally I think there is no conflict between feature work
| and micro-refactorings. If micro-refactorings are helpful for a
| feature you're working on, jump to it!
|
| Personally I've had the experience of being a Cassandra [3]
| when it comes to YAGNI; [4] maybe it is different for a junior
| dev but so often I go to a meeting and say "what if you decide
| you need to collect N phone numbers instead of 2?" and over the
| next few months there is a stream of tickets for 3, 4, 5 phone
| numbers until they finally make it N. Or a problem w/ the login
| process that is obvious to me becomes the subject of a panic
| two months later.
|
| As such I see the rule of three [5] to apply DRY is too
| conservative, it is way two common that the senior dev wrote
| two cases and then the junior dev comes in and copies it 15
| times. At least on the projects I've worked on (mainly web-
| oriented, but many involving 'intelligent systems', data
| science, etc.) people have made way too many excuses for why
| repeating themselves is good and it has had awful consequences.
|
| [1] https://en.wikipedia.org/wiki/Code_refactoring
|
| [2] the best case for Python is that a large system in some
| other language could be a small system in Python
|
| [3] https://en.wikipedia.org/wiki/Cassandra
|
| [4] https://en.wikipedia.org/wiki/You_aren%27t_gonna_need_it
|
| [5]
| https://en.wikipedia.org/wiki/Rule_of_three_(computer_progra...
| ClumsyPilot wrote:
| > More importantly, no changes over the last few years means
| that absolutely no one in the company has in-depth and current
| knowledge of how that code works, so a refactor is pointless
| because no one knows what the specific problems actually are.
| reply
|
| That's is the actual reason you might need to rewrite it. If
| your critical system is written in an ancient script that
| nobody can understand, and does no longer supported, and as a
| security risk, at some point it will simply stop working. And
| there will be nobody that can fix it.
|
| Yes, rewrite may be painful, but if you can no longer find the
| people to support the old thing it may be necessary
| poulsbohemian wrote:
| >the reality is that if there is some code in production that
| has been working unchanged for the last two years, you're
| wasting your time refactoring it.
|
| So much this. I recall watching people early in their careers
| who wanted to make their mark go after code like this that was
| just a waste of time and more likely than not to blow up things
| downstream they didn't understand. And sadly watched managers
| praise their tenacity rather than understanding the explosions
| that were being created.
| scarface_74 wrote:
| > _Do we have test coverage of the use-cases that are
| affected?_
|
| With statically typed languages and good tooling like JetBrains
| ReSharper, there are guaranteed safe automated large
| refactorings that can be done as long as someone isn't using
| reflection.m
| recroad wrote:
| I'd extend #2 to any change, not just non-trivial. It's the
| classic Kent Beck tweet, "for any change, make the change easy
| (warning: this may be hard), then make the easy change"
|
| Fixing my mental model of thinking about refactoring as a
| separate thing from "normal" development was key for me. Once I
| viewed refactoring as a thing that you do all the time as part
| of development, then I stopped even asking this question.
| sshine wrote:
| I somehow ended up doing the same. Probably as a result of
| too many failed heroic attempts at modifying read-only code
| in one big go.
|
| Now any heroic modification of code comes in a series of
| incontroversial, isolated and testably idempotent
| modifications, followed by a minimal change to business
| logic.
| thomaslangston wrote:
| I'd say #2 and #3 need to be modified.
|
| #2 Are there related changes on the horizon for the code being
| refactored?
|
| I think more qualifications than that probably miss times you
| should refactor.
|
| #3 Has the code been changing recently OR have changes been
| delayed because modifying the unrefactored code is considered
| too difficult.
|
| I've seen too many times where unrefactored code is considered
| too dangerous/difficult to modify even with total test
| coverage. Refactoring is a necessary step towards self
| documenting code in those cases.
| ninalanyon wrote:
| > 1. Do we have test coverage of the use-cases that are
| affected?
|
| And
|
| 0. Do we know which use-cases will be affected?
| rqmedes wrote:
| I regularly do large changes on many millions of lines legacy
| code bases with no unit tests. It can be done it just requires
| a lot of work. The only factor that matters is do we need to do
| this, the rest not so much
| karmakaze wrote:
| I have one reason to refactor that's served me well. Does the
| code exhibit ongoing edge/corner case bugs, which are attributed
| to the implementation not being factored in the first place?
| Another sign of this situation is exhaustive test cases which
| combine factors as they can't reliably be tested in isolation.
| Distributed logic is another common pattern, where a new feature
| is made by sprinkling bits of logic here and there, just so, and
| it all seems to do what's required. You know the kind, where
| onboarding a new person might lead to the discussion of how this
| section of your Rube Goldberg contraption works. A fancy way to
| this world is a poorly designed and maintained home-grown DSL.
|
| Continuing to build on such a 'foundation' is madness that
| spirals to code red situations: stop-the-world, no feature
| development, until shit gets stable.
| PaulHoule wrote:
| The default-oriented answer to #2 is to pass the rows through
| some function that applies the default to 'loud' if loud is
| undefined. That's really pretty code and avoids the complex if-
| statement which is a universal bad smell.
| taeric wrote:
| I like these reasons.
|
| I will offer some caution on rules for rejecting a proposal.
| Often times, the number one reason to do something is that there
| is energy there to do it. If you can take that energy and apply
| it somewhere else, by all means do so. All too often, suppressing
| some energy stops all effort.
| hylaride wrote:
| Refactoring depends heavily on circumstances. I've seen terrible
| attempts over my career for all the reasons other people have and
| will mention here.
|
| But I've also seen huge successes. Usually it boils down to "what
| are you trying to accomplish?" against current code and
| architecture. I've seen targeted refactors of micro-service code,
| including language shifts (eg python -> java where threading
| improved things a lot in that use case), as well as targeted
| shifts of code to AWS Lambdas, where execution scaling also
| benefited the use case. But in both examples there was a clear
| benefit, execution plan, and way to measure success.
|
| When you start refactoring large mono-app code bases for
| nebulous, hard to measure reasons then the risks are much higher
| (and that's even before scope creep comes into play). This gets
| even worse if the reason the current code is no longer desirable
| has reasons (both good and bad) that it is the way it is, and you
| risk recreating bad code (this was mentioned in the article) into
| new bad code. Also, migrations to new code can suck if the
| underlying dependencies, including data(bases) is just as much
| part of the problem?
| BeetleB wrote:
| Everyone talks about refactoring code. What I don't see enough
| discussion on is refactoring _tests_.
|
| It's a constant battle at work, and I tend to be firm with: "If
| the test is working, no matter how horrible the (test) code,
| leave it be."[1]
|
| For regular code, we rely on tests to validate our refactors.
| With test code, we don't have that support.
|
| If you have a bunch of tests that could do with a refactor, and
| your motivation is that you need to write more tests, then set up
| whatever abstractions you need and write the new tests, but _don
| 't_ touch the existing tests! If you really want, you can
| overtest: Write new tests with your refactored code that test the
| same thing as the old ones. It's OK to test the same thing
| multiple times (when testing is cheap).
|
| About a third of the time a coworker looks at my tests and
| refactors them, he makes an error - the most common one being not
| testing what the original test meant to test. See, the reason he
| wanted to refactor was that the original test was hard to follow.
| And _because_ of that, he failed to interpret it correctly and
| failed to properly reproduce the test in his refactored code.
|
| I then have to code review all his refactors. I have to spend
| time to figure out what my difficult-to-understand test did, and
| confirm he didn't introduce bugs. It's very, very tiring. And for
| what?
|
| And he's not a newbie. He's as good at SW as I am. Plenty of
| experience. If _he_ gets it wrong, I expect most people will get
| it wrong. With a higher error rate.
|
| This is the one case where I say: "Feel free to overcomment." If
| you took a long time understanding a given test, write out what
| you learned so that the next time you read it, you know what it
| does. I'll be happy to code review _that_.
|
| [1] Unless you are the original author of the tests, and they are
| still fresh in your mind. In that case, refactor all you want.
| kylereeve wrote:
| When refactoring tests, some form of mutation coverage [0]
| would be really nice. Verify that the tests break when the
| underlying code changes.
|
| [0] https://en.wikipedia.org/wiki/Mutation_testing
| foo_barrio wrote:
| This is a good point! We've gotten some instances of
| Chesterson's Fence that some devs casually remove during a
| "test refactor" that later allowed a regression to make it into
| prod.
|
| I've caught some errors in "test refactoring" from our multiple
| levels of testing having large overlap with teach other. Our
| end-to-end tests have a lot of overlap with the integration
| tests which in turn have a large overlap with unit testing. The
| unit tests run in a matter of seconds compared to the end-to-
| end which can take minutes or in some cases hours for our
| manual testing so the levels of testing also serve as an
| efficiency for us.
| maples37 wrote:
| For tests, when in doubt, I don't touch the original tests and
| just add my own separately. That way I know that the old tests
| still validate the originally-intended behavior, and my new
| tests validate what I expect them to do.
|
| If the tests aren't broken, is there ever a good reason to make
| sweeping changes to them?
| martin-t wrote:
| Sounds like the tests would benefit from comments explaining
| what they are testing and why. Just a few days ago there was an
| article from an experienced dev that after multiple decades he
| thinks you simply can't put too many comments in tests. I was
| on the fence with it but seeing your comment, I now know where
| it comes from.
|
| Maybe this could also be solved with better tooling - tests
| that allow substituting the tested system with a dummy that
| fails and thus verify the test is actually working.
| GiorgioG wrote:
| Does anyone really get time to refactor anything these days?
| Seems like (at least where I'm at), everything is go-go-go,
| feature work.
| foo_barrio wrote:
| I try to never ever use the word "refactor" or "clean up" in
| any of my work items or even commit messages. My boss and other
| teams I am collaborating with are like clients to me.
| Restaurant bills typically do not include explicit line items
| for "dish washing, mopping, grease-trap cleaning, oil disposal,
| etc."
|
| The downside is that other people/teams can appear to work much
| faster and put you at a disadvantage or even risk of being let
| go due to this perception. Unfortunately this is where IMO you
| have to play the game and make sure you toot your own horn to
| the right people sometimes.
| maccard wrote:
| If you work in a kitchen, you're expected to keep the kitchen
| clean. The trick to that is knowing how to efficiently clear
| up, and being prepared for the waves when they come.
|
| As an engineer it's no different. If you're not keeping in top
| of your own debt, you're actually falling behind.
| IshKebab wrote:
| Yeah just do it as part of your work without telling people.
| You're paid to write software and part of writing software _is_
| refactoring.
| merb wrote:
| When working with git it's better to split refactorings, bug
| fixes and features per commit or merge request (if you squash)
| else your git history will be not so useful anymore.
| gspencley wrote:
| > We often reach a point during refactoring where it seems like
| there is an easy improvement that applies to almost all cases.
| It's usually better not to impose additional abstraction if it
| only matches "almost" all cases.
|
| Need to nit-pick on this one. There are design patterns,
| depending on your programming paradigm, that enable you to de-
| dupe the common stuff while allowing for all of those little
| variances to do their own unique thing without duplicating
| anything.
|
| In OOP the Template Method pattern comes to mind.
|
| I would even go so far as to say that the "almost all cases"
| problem is such a common problem in software development, that
| the patterns world has come up with various solutions to that
| which are intended to simplify, not complicate.
|
| I'm sure there are concrete examples where there are no good pre-
| existing solutions, and there are often examples where cures can
| be worse than the diseases and you need to make a judgement call.
| But don't give yourself an excuse to duplicate code because you
| don't know how to de-dupe in the "almost all cases" scenario.
| That's a very solved problem.
| poulsbohemian wrote:
| I spent about a 12 year period of my career doing triage and app
| performance work. What struck me was how often the core problems
| were architectural. Either the design was bad from the beginning,
| or the requirements had changed so dramatically over time (think
| scaling, for example) that the architecture no longer worked. I
| bring this up because while I saw a lot of questionable code, at
| the micro level it really didn't matter. Sure it could have been
| refactored and improved, but that would have been essentially
| diminishing returns. Often the improvements that were needed
| would have been so painful that rather than make them, I watched
| companies spiral around either throwing hardware at problems or
| replacing the application entirely because that kind of Bandaid
| rip was deemed "easier" in the overall politics of the corporate
| world. So point being - sure, refactor what you can, but don't
| get too hung up on things that ultimately won't matter.
| bob1029 wrote:
| If it won't impact your margins or the customer's willingness to
| pay for the product then it is difficult to justify.
|
| I've been involved with (and responsible for) many "developer
| aesthetic" refactors over the years. They feel good in the moment
| but after blowing 2-3 weeks with regression testing, hot patches,
| etc., you start to wonder if it was all worth it.
|
| There is stuff that is properly nasty and needs to be dealt with,
| but if you are spinning your wheels on things like namespaces
| being stuck on old company/product names, I would just give it up
| and move on. The average customer cannot see any of the things
| that frustrate us unless they are being done _very_ poorly.
| magicalhippo wrote:
| We've got a lot of 20+ year old code in production, a lot
| written late at night in a crunch. Much of it's ugly, non-
| optimal and screams to be ripped out and replaced.
|
| But it is working, in areas that see little change and are not
| overly performance sensitive.
|
| So we only do something with it if we need larger changes.
| Otherwise we leave it be and spend our time being productive
| elsewhere.
| tikhonj wrote:
| > _There's no benefit to improving code that never changes, even
| if it is highly complex._
|
| Only if you don't care about being able to understand and debug
| the code in the future!
|
| And even then it's if you're confident in predicting how much
| code will or won't need to change, which, well...
| mason55 wrote:
| The point is not to improve it until you actually need to
| change it.
|
| Like you said, you're probably bad at predicting which code is
| going to need changes in the future, so if it's working now and
| you don't need to change it then it's a silly risk to try to
| improve it right now.
| patrick451 wrote:
| There are two conflicting rules of thumb floating around here
|
| 1. If hasn't changed in a long time and no business requirement
| on the horizon will force it to change, don't refactor. 2. Code
| is read more than written, so optimize for readability.
|
| (1) can only be followed to the detriment of (2), unless you add
| the condition that the code is _also_ code which developers never
| need to read and understand to implement other features.
| gbuk2013 wrote:
| > If we're embarking on a change that is not really refactoring
| (for example looking at a bug or an adjustment after a third
| party change), we can't fix it with refactoring.
|
| Funnily enough I did once fix a bug through refactoring. IIRC it
| was some intermittent race condition in some async code that I
| just couldn't figure out, so I rewrote the code to do the same
| thing but simpler and the problem disappeared. I was too tired
| and relieved to continue figuring it out why it was originally
| broken.
| TiredGuy wrote:
| Same. Sometimes the best way to fix a bug is to refactor it out
| of existence.
| Salgat wrote:
| And if you're lucky a new hidden bug appears.
| mont_tag wrote:
| One more consideration is whether a team of people have invested
| time building a mental map of the current code. Perhaps some
| rearrangement would be better, faster, clearer, or have some
| other virtue. But that benefit has to be balanced against
| invalidating the mental caches of all the other team members.
|
| More than once, I've studied code and become adept at maintaining
| it. Then someone "refactors" everything and I have to invest the
| effort again. After a few cycles of this, I lose interest in ever
| seeing the code again. At that point, I can no longer orient a
| new dev to the code because I'm no longer oriented myself.
| kevmojay138 wrote:
| > Instead of "refactoring" without tests, we can start by writing
| some tests!
|
| I agree with this but I have always struggled with putting it
| into practice. Good code is easy to test. Often, I want to
| refactor to make it easy to test.
| einpoklum wrote:
| My top reasons not to refactor:
|
| 1. My boss won't let me. The usual excuse: "Too risky... there
| are other priorities... and besides, where are those performance
| improvements you promised me? We don't pay you to make the code
| pretty."
|
| 2. Boss of group which owns some of the code I need to refactor
| won't let me: "Too risky... we have other priorities... have your
| boss bring this up in the next [bogus] meeting"
|
| 3. Other developers get panties in a bunch over a tiny refactor
| (semantically) but which involves changes to names in a lot of
| files.
|
| 4. Knowing that if I'd start this refactor I will not be able to
| resist another refactor until I'm basically refactoring people's
| entire code to look quite different.
| einpoklum wrote:
| Peeves about the items in the linked post:
|
| > 1. The change is not really a refactor
|
| Is anything ever really just a refactor, though?
|
| > 3. Another refactor is already in progress
|
| Ah, but - in large and poorly-maintained codebase, it is always
| the case that some refactoring is already in progress, likely
| stalled for weeks, months or years as priorities have shifted
| without it having been completed.
|
| > 4. The code is unlikely to change
|
| If the code is unlikely to change, there's a good chance you
| might want to refactor it out of your repository anyway, into
| some low-frequency-of-changing library.
|
| > There's no benefit to improving code that never changes
|
| Oh, there are lots of benefits to improving code that never
| changes! And that's because other code _uses_ the code which
| never changes. And if originally the use is clunky, but you
| manage to improve it by your refactor, youve' done well indeed.
|
| > 5. There are no tests
|
| A repository which lacks test coverage is extremely likely to be
| in dire need of all kinds of refactoring. Waiting until someone
| (maybe even yourtself) writes tests for most code, before doing
| anything in the mean time - well, it basically means giving up on
| yourself.
|
| > we can start by writing some tests!
|
| Well, that's nice, but - if we do that, then no refactoring would
| get done, and we'll have to write code based on ugly and unweildy
| older code, which only adds stuff to refactor later. Plus, who's
| to say the tests don't fail _already_? :-(
| Fishkins wrote:
| > Ah, but - in large and poorly-maintained codebase, it is
| always the case that some refactoring is already in progress,
| likely stalled for weeks, months or years as priorities have
| shifted without it having been completed.
|
| This is true, and something I also thought of when reading that
| point. I don't think it's necessarily a counterargument,
| though. It's probably a better idea to spend your time helping
| to complete the previous refactor instead of starting your new
| one. Codebases in which many refactorings are started but not
| completed can be worse than ones that aren't refactored at all.
|
| There could be exceptions if your new changes is very small,
| localized, and unlikely to interfere with the other changes
| going on.
| madmountaingoat wrote:
| It's all solid advice but that exchange in #5 that the author
| found delightful, drives me bonkers. If you talk to the people on
| your team like that, you're not being helpful, you're being an
| a-hole. I realize its just making a point in the context of the
| article, but this industry seems to collect pedants, so it seems
| worth pointing out as an anti-pattern.
| xg15 wrote:
| > _"Before we can add tests, we need to refactor." - "No." -
| "Refactoring th-" - "No." - "Refa-" - "No."
|
| What to do instead: Instead of "refactoring" without tests, we
| can start by writing some tests! Writing new tests will help us
| learn about the code. After that, we can come back to
| refactoring._
|
| I assume the author means system tests here? Because it's
| absolutely possible to have code that is so entangled that it's
| impossible to write self-contained unit tests before, well,
| refactoring the code.
___________________________________________________________________
(page generated 2025-02-06 23:01 UTC)