I am faced with a class that is 13,000 lines long.
I know by itself isn't wrong but it is a controller for a very complicated window, and every single command is inside this one class.
The trouble is that now the class has become spaghetti, with all the commands calling utility functions using the same fields and calling methods on the super class. Methods called "checkXYZ" having side effects etc.
So how do people go about refactoring this type of code?
Oh of course there are 0 tests, so all changes have to be very safe.
Write unit tests as much as possible. That might be difficult with a GUI controller, but do your best with it. A colleague of mine had to maintain a 10,000 line method that computed a return on investment given many possible inputs. The original developer had literally gone insane, so he wasn't available to answer questions. My colleague started by writing test cases and eventually had nearly 1000. He could thus be confident that his refactoring didn't break anything.
Java switch statements are simple short-hand ways to write if statements. This would be only a trivial change and certainly not the refactoring being sought, in my opinion. The conditional logic is needed period.
The first step to reengineering an application would be to clearly define the reasons why. You should have a good reason to reengineer and you should be able to clearly define concrete problems or limitations in the existing application. If there are no problems and there are no limitations, then it may be best to leave the application as it is.
If there are problems or limitations, then the first step is to document them in a technical document. Once you have them clearly written down, then you can start to think about ways to redesign them. It is also helpful to document what business requirements are related to the problems or limitations. This will help you clearly think about the effect of any design changes. Programmers tend to freely hallucinate about code and theory, so its best to keep this in check.
Stephan van Hulst
Joined: Sep 20, 2010
What do you mean? Can't you just create an enum constant for each command with an associated String?
Then you could do two things. You can make either a big switch statement with all the enum constants as cases, or you can give each constant a constant specific class body that implements some abstract perform() method.
Normally I would advocate the latter option, but a switch statement with simple cases may be easier to read:
And yes, like Jimmy implies this would mostly be an exercise in making things read a little bit easier. There are no big structural changes involved.
Maybe you can provide us with more information on what the system is supposed to do, and why exactly it's such spaghetti?