This week's giveaway is in the Android forum.
We're giving away four copies of Android Security Essentials Live Lessons and have Godfrey Nolan on-line!
See this thread for details.
The moose likes OO, Patterns, UML and Refactoring and the fly likes Refactoring very old spaghetti Big Moose Saloon
  Search | Java FAQ | Recent Topics | Flagged Topics | Hot Topics | Zero Replies
Register / Login


Win a copy of Android Security Essentials Live Lessons this week in the Android forum!
JavaRanch » Java Forums » Engineering » OO, Patterns, UML and Refactoring
Bookmark "Refactoring very old spaghetti" Watch "Refactoring very old spaghetti" New topic
Author

Refactoring very old spaghetti

Wendy Gibbons
Bartender

Joined: Oct 21, 2008
Posts: 1107

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.

here is an example:
Stephan van Hulst
Bartender

Joined: Sep 20, 2010
Posts: 3605
    
  14

Well, first off I would probably make an enum for all those commands. I would also turn all those horrible if else statements into a switch statement.
Greg Charles
Sheriff

Joined: Oct 01, 2001
Posts: 2841
    
  11

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.
Wendy Gibbons
Bartender

Joined: Oct 21, 2008
Posts: 1107

Stephan van Hulst wrote:Well, first off I would probably make an enum for all those commands. I would also turn all those horrible if else statements into a switch statement.

unfortunately it is a string controlling it.
Jimmy Clark
Ranch Hand

Joined: Apr 16, 2008
Posts: 2187
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.

Good luck!
Stephan van Hulst
Bartender

Joined: Sep 20, 2010
Posts: 3605
    
  14

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:


[edit]

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?
Wendy Gibbons
Bartender

Joined: Oct 21, 2008
Posts: 1107

Wendy Gibbons wrote:
Stephan van Hulst wrote:Well, first off I would probably make an enum for all those commands. I would also turn all those horrible if else statements into a switch statement.

unfortunately it is a string controlling it.

sorry i get confused, i was thinking enum and switch 2 seperate jobs, and i couldn't do the switch as it is currently a string.
 
I agree. Here's the link: http://aspose.com/file-tools
 
subject: Refactoring very old spaghetti
 
Similar Threads
Recursive method and an array
Displaying line # with JTextArea
Help with printf()
Multiple files?
Refactoring - consequences of changing methods to static