Maybe this is somehow useful to you, I will present one way I work on legacy code. There are many more like extracting fields first. It depends, it is different for each piece of legacy code. If it doesn't work the first time, go back one hour (revert changes), try it a second and even a third time, you will get experience at that, under which conditions to start that way or another.
One working way is:
- At the first step you should categorize the methods you have, e.g. business, controller, data, ui and sort them within the class to group them. If a method can not be categorized properly, it does to much: Extract smaller methods. categorize these methods too.
- At the second step I try to reduce dependencies from the class, where I want to move code away from. You categorized those offending methods, and all these methods should in best case have no dependencies to the class fields or members. This can be done by introducing parameters instead of using class fields into method-signatures. The resulting new method can be potentially moved away.
- Third step is to create a new class where one of the categories belong to (in java I'm using inner classes at first and then extract the full class into a separate file, when all depencies are using a public contract) and to create an instance of the new class in the class to clean up.
- Now move the easiest method (lowest hanging fruit) into the new class and replace the invocation with an invocation on the new created object.
- All these refactorings are meant to cause no differences in runtime execution order. Only apply those refactorings. Try to rely on IDE-Support and double check any change, since you do not have unit-tests yet. When you do small step refactorings, you can stop whenever you need/have to - and deliver the code as is.
- Commit often. I mean all one or two minutes. Keep all changes as little as possible and do not try to be clever - no parallel refactorings - one after the other - write a list of steps down - do not try to remember all of them - all it needs to disturb you is a phone call - and your efford is wasted. Do not refactor 10 minutes without commiting. These refactorings are error-prone. Committing often enables you to find the cause of a refactoring error e.g. via binary search on commits.
- Now create unit tests for the one single extracted and moved method.
- Now move the next low hanging fruit method to new class, write tests etc.
- if you have dependencies to the previous class, make these dependencies temporarely public, annotate/document them, that this is not the public API, but needeed for refactoring.
- now check, if you need interfaces and extract them to reduce the depencencies between the former class and the new extracted class.
This will step by step remove the code safely from the class. But do not move invocations around within method code, only work with/on full methods. (Optimizations will come next when you have more test coverage)
Now you have unit tests for the new extracted methods and the former class code code is less cluttered. Now you will experience more methods which may belong to the extracted class also. Since you see, who is now invoking the new extracted class. Write more tests.
Proceed with other categories. If you get stuck - it is fine, return another time or sleep a night over it. This will improve the code step by step and no step is expensive at all - only one to two minutes. When you get more experience on this, improve the higher hanging fruits. Start with the lowest hanging ones. Before you add a new feature, try to declutter the code, try to identify the code you need to change and separate it from the code you do not need to touch. Then add new methods using a seam and write test for them.
If you might want a suggestion on a book to read - on legacy code: "Working Effectively with legacy code" from Michael C. Feathers. Also do not miss the great youtube video "Practical Refactoring" on refactoring in very small steps. It is somehow eye opening. But you do not have to agree to them fully.
TL;DR: Well after reading this again this is working on SRP (Single Responsibility Principle) first.
hth a bit.
