Thursday 12 June 2014

Getting the most out of Extract Class

Resharper is a wonderful tool. I can't imagine working in the horribleness of legacy code without it.

Every so often you come across a little workflow that makes slicing and dicing code either. For example, before you could "Move Instance Method" you could "Make Static", "Move Method" and "Make instance method". Knowing you could do this made tearing code apart easier.

Recently I've been using "Extract class" a million and one times to deal with one of those 10K line long classes that no-one ever admits to having. The classes in question are in this:

class DoesEverythingAndThenSome {

       private ThingToDoWithA1 m_ThingToDoWithA1;
       private ThingToDoWithA2 m_ThingToDoWithA2;
       private ThingToDoWithA3 m_ThingToDoWithA3;

       private ThingToDoWithB1 m_ThingToDoWithB1;
       private ThingToDoWithB2 m_ThingToDoWithB2;
       private ThingToDoWithB3 m_ThingToDoWithB3;

       // repeat for thousands of other "things"

       public void DO_ALL_THE_THINGS_WITH_A () {

       }
   
       public void DO_ALL_THE_THINGS_WITH_B () {

       }

       // thousands of lines of random shit
       public void example_of_random_shit () {
          if (incrediblyComplicatedCondition()) {
             for (var apples in bananas) {
               m_ThingsToDoWithA1.SomethingImportant();
             }
          } else if (auberginesAreLumpy()) {
             m_ThingsToDoWithB.SomethingElse();
          }
       }
    }

Obviously I want to extract out the responsibilities to do with A and B into separate class. Using "Extract Class" directly doesn't work because I can't pull out the references in example_of_random_shit without making properties public and introducing back references from the extracted class to the parent class.

The simple refactoring is to just extract out each line to do with each field into a single method.

class DoesEverythingAndThenSome {

       /* the rest is the same as above */

       public void SomethingToDoWithA() {
           m_ThingsToDoWithA1.SomethingImportant();
       }
 
       public void SomethingToDoWithB() {
           m_ThingsToDoWithB.SomethingElse();
       }

       // thousands of lines of random shit
       public void example_of_random_shit () {
          if (incrediblyComplicatedCondition()) {
             for (var apples in bananas) {
                 SomethingToDoWithA();
             }
          } else if (auberginesAreLumpy()) {
             SomethingToDoWithB();
          }
       }
    }

Once I've completed this simple refactoring, "Extract Class" can now do the heavy lifting and I can move all the fields, and all the functions across in a single refactoring. What was previously hard (unpicking the back references) is now incredibly simple and I end up with a mechanical transformation to get data and functions in the right place. Extract class will now give me:

class ThingsToDoWithA {
       private ThingToDoWithA1 m_ThingToDoWithA1;
       private ThingToDoWithA2 m_ThingToDoWithA2;
       private ThingToDoWithA3 m_ThingToDoWithA3;

       // snip constructor

       public void DO_ALL_THE_THINGS_WITH_A () {

       }

       public void SomethingToDoWithA() {
           m_ThingsToDoWithA1.SomethingImportant();
       }
    }

    class DoesEverythingAndThenSome {
       
       private ThingToDoWithA m_ThingToDoWithA;

       private ThingToDoWithB1 m_ThingToDoWithB1;
       private ThingToDoWithB2 m_ThingToDoWithB2;
       private ThingToDoWithB3 m_ThingToDoWithB3;

       // snip constructor

       // repeat for thousands of other "things"
   
       public void DO_ALL_THE_THINGS_WITH_B () {

       }

       // thousands of lines of random shit
       public void example_of_random_shit () {
          if (incrediblyComplicatedCondition()) {
             for (var apples in bananas) {
               m_ThingsToDoWithA.SomethingImportant();
             }
          } else if (auberginesAreLumpy()) {
             m_ThingsToDoWithB.SomethingElse();
          }
       }
    }

Repeat this mechanically for all the other responsibilities Then the hard work begins of actually working out sensible names...