harriyott.com

A new refactoring please

I use the built-in refactoring in Visual Studio for my C# code. I often use "Extract Method" to cut out a chunk of code from the middle of a growing method. This creates a new method under the original, moves the selected code into the new method, and replaces it with a call to the new method, working out what the parameters should be.

So, this contrived example:

        public void HaveAGoIfYouThinkYoureHardEnough(Person queueJumper)

        {

            bool hardEnough;

            int currentHardness = 0;

            if (queueJumper.Height >= 6)

            {

                currentHardness += 2;

            }

            if (queueJumper.Weight >= 14)

            {

                currentHardness += 3;

            }

            if (queueJumper.Weight >= 18)

            {

                currentHardness += 2;

            }

            if (queueJumper.Weight <= 8)

            {

                currentHardness -= 4;

            }

            if (queueJumper.ScarCount > 2)

            {

                currentHardness += 1;

            }

            if (currentHardness > 5)

            {

                hardEnough = true;

            }

            else

            {

                hardEnough = false;

            }

            if (hardEnough)

            {

                LetHimHaveAGo(queueJumper);

            }

        }



becomes

        public void HaveAGoIfYouThinkYoureHardEnough(Person queueJumper)

        {

            bool hardEnough;

            int currentHardness = 0;

            currentHardness = AddHeightHardness(queueJumper, currentHardness);

            currentHardness = AddWeightHardness(queueJumper, currentHardness);

            currentHardness = AddScarHardness(queueJumper, currentHardness);

            hardEnough = IsHardEnough(currentHardness);

            if (hardEnough)

            {

                LetHimHaveAGo(queueJumper);

            }

        }

 

        private static bool IsHardEnough(int currentHardness)

        {

            bool hardEnough;

            if (currentHardness > 5)

            {

                hardEnough = true;

            }

            else

            {

                hardEnough = false;

            }

            return hardEnough;

        }

 

        private static int AddScarHardness(Person queueJumper, int currentHardness)

        {

            if (queueJumper.ScarCount > 2)

            {

                currentHardness += 1;

            }

            return currentHardness;

        }

 

        private static int AddWeightHardness(Person queueJumper, int currentHardness)

        {

            if (queueJumper.Weight >= 14)

            {

                currentHardness += 3;

            }

            if (queueJumper.Weight >= 18)

            {

                currentHardness += 2;

            }

            if (queueJumper.Weight <= 8)

            {

                currentHardness -= 4;

            }

            return currentHardness;

        }

 

        private static int AddHeightHardness(Person queueJumper, int currentHardness)

        {

            if (queueJumper.Height >= 6)

            {

                currentHardness += 2;

            }

            return currentHardness;

        }



So I've added four more methods to make the first method smaller. Mostly, this type of new method will not be used by any other method, as I extract them for readability, not functionality.

If this main method (and it's four extracted methods) was called from an even mainer method, which did something else, then the two main methods (which originally were adjacent) become separated:

        public void GetIntoNightClub()

        {

            HaveAGoIfYouThinkYoureHardEnough(new Person("John Prescott"));

            WillTheBouncerLetMeIn();

        }



After refactoring, HaveAGoIfYouThinkYoureHardEnough and WillTheBouncerLetMeIn now have four methods between them.

I generally don't want to see these four methods again, and as I've made the method names self-commenting, I won't need to refer to them unless there's a bug. As there's only four static methods, and they're related to the existing method, creating a new class for them is probably not the right way to go.

Therefore, I propose two new refactorings. The first, and easiest, would be to add a region under the main method, which contains the extracted methods:

        #region Methods extracted from HaveAGoIfYouThinkYoureHardEnough

        // ...

        #endregion



Let's call this "Extract method to region". The region can be collapsed, and both main methods are visible at once.

The second refactoring would be to shuffle the methods off to another file, using the cunning partial class feature of C#. Let's call this "Extract method to new file". The file will be called <ClassName>_HaveAGoIfYouThinkYoureHardEnough.cs, so it's easy to see in the solution explorer what the file contains. Obviously the "Rename" refactoring would have some extra work to do here.

So, Visual Studio refactor team, please get to work...
6 July 2006