One of the first apps I built for my company is still in use today and occasionally gets new features. It's both an awesome and terrifying feeling to know that something I built is active and used daily. I've learned a lot since I built that app and I've been able to fix and update a lot of the things I did "wrong" in the beginning.
However most of the "fixes" have been on a line or a query or just splitting a long method into 2. The code base itself needs a facelift and a reorganization. I can see the problems but I don't know where to start in terms of moving large blocks of code around. My test coverage is pretty good so I'm not worried about breaking things as much as I'm worried about doing it efficiently and in small enough chunks that if we get busy on other projects I can easily come back.
Anyone have any tips or stories about refactoring they would like to share?
Top comments (16)
This is a big one for me. I have been the only dev on this project since its inception but we're looking to bring on another dev soon and I'm kinda embarrassed about some of the code and how its organized (or lack thereof). I understand that my newer projects are a lot
bettercleaner and that this was a massive undertaking for someone of my skill level at the time but I'm still not comfortable with showing the code to someone else.Always!
Thanks for the tips, good to know I'm on the right path.
Ah ah I understand. But see it this way: you may not have a choice soon. And with the right person and some pair programming to teach them the code base you might both learn something
That's the hope!
If you have about two hours, I would recommend watching this youtube video:
youtu.be/aWiwDdx_rdo?t=3176
They discuss a technique where they can continuously improve the code without having to understand the code!
This does require that you have an adequate suite of tests to back you up.
The Technique
They did not take the time to understand any of the code: by slowing improving names and removing duplication, they were able to make the program more maintainable.
They eventually discovered the IChart abstraction, which made adding new chart types to the application a safe operation, since existing chart types methods would no longer need to be modified (which was their original goal).
At any point, they could have stopped working and deployed the code, since they worked in two-minute intervals and committed changes at every safe point (the code was cleaner at every check in, than when it was originally checked out).
You may also be interested in reading one of J.B. Rainsberger's articles on simple design.
blog.thecodewhisperer.com/permalin...
The article explains how improving names and removing duplication cause abstractions to surface, which allows responsibilities to be redistributed among classes. The video above is definitely a prime example of this process in action.
The four rules of simple design can be used to determine when you should strike the metal.
Beyond those tips, start slow and refactor only the pieces that you are working on currently. This will limit the amount of risk you introduce with each set of changes. This also prevents your boss from asking the question: why did feature X break, when no one worked on that feature in this deployment.
Luckily, given that it's all my code I understand the code pretty well but I'll still check the video out, thanks! I'll definitely check out the article on simple design too, I'm about halfway through POODR right now. Thanks for the resources!
In my experience, it helps to work outside-in. Is there a way to scope the refracturing you want to do into pieces that break down into individual tasks? Doing so will help keep you organized, and perhaps break the refectory down into sprint-able chunks that can be done/deployed alone.
For the rails side I'm trying to stick with each part as M and C. One of my main goals is to extract some of the more presentational code from the models into helpers and skinny up the controllers.
Word. That could be an epic in itself, yeah? “Abstract presentational code to helpers” and have stories for each model/model type. That way, if you work in chunks you can CI your changes.
Since rails is serving as only an api in this instance I figured I could use decorators pretty heavily in the json responses and really DRY up both the jbuilder files and my models.
What I would like to share is that many times I ended up with a recursive mess of refactoring steps that I did not understand anymore. I was trying to do too much, too fast, too deep at once. And I could not back-track.
That having been the case, I would like to recommend the following:
Especially for the last point, but also aiding the others, I would recommend you use git & branches. Open a branch for your refactoring activities, commit as soon as you are happy with your refactoring step. If you land in a "blind alley" you will be able to go back.
Hope that helps, and have fun! Refactoring can be a very rewarding activity.
All great points! I completely agree
Do you have unit tests for critical functionality or complex conditional logic? Might want to start from there. It might start out really hard depending on your architecture, but once you've set it up, you should be able to refactor fast. Think of it as setting up a safety net before doing a paint or window job on a really tall building. You'll move faster if you aren't worried about shit happening.
Yes, luckily the core of the app is pretty well covered with tests and they have been a huge help in making sure refactoring doesn't break things (which it has, repeatedly)
If we consider projects like a tree of components, then it's best to work your way from the leaves to the core, refactoring the logic to a more central location whether it is the parent component or into a service... This is also a good method to slowly convert from a service centric structure to a more reactive where you might be integrating redux or similar.
Of course, it's best to consider the core or the end of the journey from the leaves before beginning to refactor and try to focus on a single trail so that when staying from another leaf you might find that some of the logic it uses, or could use, you already refactored to a central/parent location.
But often one doesn't get to have that kind of deliberate flow due to timing or having to sneak it into new feature completion... In that case,I would just focus on the singular or internal logic, looking for unused variables, dead code, reusable code and potential memory leaks that do not have an effect on parent or sibling components, or very minimally at least.
When I was refactoring the frontend (Vue.js) I moved a lot of the code into vuex and it definitely helped make the components and views A LOT easier to work with.
But of course deadlines get in the way, I think you're right about about looking for dead code first. Might make it easier to approach the actual refactor.