Refactoring large modules using Eclipse

I have recently had to consider refactoring a piece of Java which had got slightly out of hand – the module was 800 lines long and the if statements so deeply nested that they ran well off the right-hand edge of the page. I will NOT identify where it came from or to criticize – I have written much worse in my past (you can do really fun things with computed GOTOs in FORTRAN.). But it was and is unmaintainable and we care about that in the Centre.
So I thought that I would sit down with Eclipse in front of the football and refactor it. Eclipse has this really neat Refactor that allows you to select a chunk of code and turn it into a method. For example:
public void add3DStereo() {
// StereochemistryTool stereochemistryTool = new
// StereochemistryTool(molecule);
ConnectionTableTool ct = new ConnectionTableTool(molecule);
List cyclicBonds = ct.getCyclicBonds();
List doubleBonds = molecule.getDoubleBonds();
for (CMLBond bond : doubleBonds) {
if (!cyclicBonds.contains(bond)) {
CMLBondStereo bondStereo3 = create3DBondStereo(bond);
if (bondStereo3 != null) {
bond.addBondStereo(bondStereo3);
}
}
}
List chiralAtoms = new StereochemistryTool(molecule).getChiralAtoms();
for (CMLAtom chiralAtom : chiralAtoms) {
CMLAtomParity atomParity3 = null;
atomParity3 = calculateAtomParity(chiralAtom);
if (atomParity3 != null) {
chiralAtom.addAtomParity(atomParity3);
}
}
}
I now select the first for loop and turn it into a method; and repeat for the second and get:
public void add3DStereo() {
// StereochemistryTool stereochemistryTool = new
// StereochemistryTool(molecule);
ConnectionTableTool ct = new ConnectionTableTool(molecule);
List cyclicBonds = ct.getCyclicBonds();
List doubleBonds = molecule.getDoubleBonds();
addBondStereo(cyclicBonds, doubleBonds);
List chiralAtoms = new StereochemistryTool(molecule).getChiralAtoms();
addAtomParity(chiralAtoms);
}
/**
* @param chiralAtoms
*/
private void addAtomParity(List chiralAtoms) {
for (CMLAtom chiralAtom : chiralAtoms) {
CMLAtomParity atomParity3 = null;
atomParity3 = calculateAtomParity(chiralAtom);
if (atomParity3 != null) {
chiralAtom.addAtomParity(atomParity3);
}
}
}
/**
* @param cyclicBonds
* @param doubleBonds
*/
private void addBondStereo(List cyclicBonds, List doubleBonds) {
for (CMLBond bond : doubleBonds) {
if (!cyclicBonds.contains(bond)) {
CMLBondStereo bondStereo3 = create3DBondStereo(bond);
if (bondStereo3 != null) {
bond.addBondStereo(bondStereo3);
}
}
}
}
The whole thing took 30 seconds, including choosing the module names. Eclipse did all the params, documentation return values – everything.
Try it – it will really fix up many sorts of grotty code…