Refactoring with CodeRush Xpress

One of the first things I do with a new install of Visual Studio(VS) is I head over to DevExpress and download/install CodeRush Xpress. CodeRush Xpress is a free refactoring tool from DevExpress for VS 2010 or older(They stopped offering the free version for VS 2012 or newer so now it looks like the first thing I'll be doing with a new install of Visual Studio is beg my boss for a CodeRush license).

If you are not familiar with refactoring and you are interested in being a better coder you should read Martin Fowler's book: Refactoring: Improving the Design of Existing Code (Addison-Wesley Object Technology Series). In it the authors Martin Fowler, Kent Beck, John Brant, William Opdyke and Don Roberts give very practical advice on improving the design of code.

Along with improving the design of code, refactoring can improve the readability of code.  My favorite refactoring pattern for this is Extract Method(EM). When you come across a really long method or find that in a moment of coding frenzie you have created one yourself, it's time to do some EM'ing to make things more readable for the next person that has to look at your code(which may be yourself in which case you will hear the thanks first hand from yourself).

For example take this code I found on an open source project:
  Private Function UpdateHintPath(ByVal projfile As String) As Boolean 

        Dim xmldoc As XmlDocument 
        Dim xnl As XmlNodeList 
        Dim xelement As XmlElement 
        Dim hintpathAttr As String = String.Empty 
        Dim newHintPathAttr As String 
        Dim name As String = String.Empty 
        Dim assmbname As String = String.Empty 
        Dim refinclude As String 
        Dim projGUID As String = String.Empty 
        Dim blDoSave As Boolean = False 
        Dim blUpdateHintPathOk As Boolean = True 
        Dim elementToRemove As New Hashtable 
        Dim attrToRemove As New ArrayList 
        Dim ien As IEnumerator 

        File.SetAttributes(projfile, FileAttributes.Normal) 

        xmldoc = New XmlDocument 
        xmldoc.Load(projfile) 

        If GetDotNetVersion(projfile) = DOTNETVERSION.TWO Then 

            'Get the namespace URI of the projects 
            Dim projectroot As XmlNode = xmldoc.ChildNodes(0) 
            If projectroot.Name.Equals("xml") Then 
                projectroot = xmldoc.ChildNodes(1) 
            End If 
            Dim projectNameSpaceURI As String = CType(projectroot, System.Xml.XmlNode).NamespaceURI 
            Dim nsmgr As XmlNamespaceManager = New XmlNamespaceManager(xmldoc.NameTable) 
            nsmgr.AddNamespace("proj", projectNameSpaceURI) 

            'Remove all project references  - We need to remove the project reference becuase as UBuild build at project level and not sln 
            'if one of the referred project is not already build but is a dependent of the main project being built it will cause the build to fail 
            'So we take out the project reference. 

            xnl = xmldoc.GetElementsByTagName("ItemGroup") 

            For Each xelement In xnl 
                If xelement.HasChildNodes Then 
                    Dim childNodeList As XmlNodeList = xelement.ChildNodes 
                    For Each _child As XmlNode In childNodeList 
                        If _child.NodeType = XmlNodeType.Comment Then 
                            Continue For 
                        End If 
                        Dim child As XmlElement = DirectCast(_child, XmlElement) 
                        If child.Name.Equals("ProjectReference") Then 

                            If Not elementToRemove.Contains(child) Then 
                                elementToRemove.Add(child, xelement) 
                            End If 

                            If child.HasChildNodes Then 
                                If child.GetElementsByTagName("Project").Count = 1 Then 
                                    projGUID = child.GetElementsByTagName("Project").Item(0).InnerText 
                                End If 
                                If child.GetElementsByTagName("Name").Count = 1 Then 
                                    assmbname = child.GetElementsByTagName("Name").Item(0).InnerText 
                                End If 
                            End If 

                            'assmbname = child.ChildNodes(0).InnerText 
                            'projGUID = child.ChildNodes(1).InnerText 

                            Dim newelement As XmlElement = xmldoc.CreateElement("Reference", projectNameSpaceURI) 
                            Dim xmlName As XmlElement = xmldoc.CreateElement("Name", projectNameSpaceURI) 
                            Dim xmlHintPath As XmlElement = xmldoc.CreateElement("HintPath", projectNameSpaceURI) 

                            xmlName.InnerText = CType(htProjGuidAssmb(projGUID), String) 
                            xmlHintPath.InnerText = buildFileLocation & CType(htProjGuidAssmb(projGUID), String) & ".dll" 
                            newelement.SetAttribute("Include", CType(htProjGuidAssmb(projGUID), String)) 

                            newelement.AppendChild(xmlName) 
                            newelement.AppendChild(xmlHintPath) 

                            xelement.AppendChild(newelement) 

                            xmldoc.Save(projfile) 

                            If Not attrToRemove.Contains(newelement) Then 
                                attrToRemove.Add(newelement) 
                            End If 

                        End If '//If child.Name.Equals("ProjectReference") 

                        'Find <Reference> element is present without any child node 
                        'Remove if it is of type <Reference Include="[AssemblyName], Version=1.0.915.2008, Culture=neutral, processorArchitecture=MSIL" /> 
                        If child.Name.Equals("Reference") And child.HasAttribute("Include") And Not child.HasChildNodes Then 

                            refinclude = child.GetAttribute("Include") 

                            If InStr(refinclude, ",") Then 

                                assmbname = child.GetAttribute("Include").Split(","c)(0) 

                                If Not ToBeExcluded(assmbname) Then 

                                    If Not elementToRemove.Contains(child) Then 
                                        elementToRemove.Add(child, xelement) 
                                    End If 

                                    Dim newelement As XmlElement = xmldoc.CreateElement("Reference", projectNameSpaceURI) 
                                    Dim xmlName As XmlElement = xmldoc.CreateElement("Name", projectNameSpaceURI) 
                                    Dim xmlHintPath As XmlElement = xmldoc.CreateElement("HintPath", projectNameSpaceURI) 

                                    xmlName.InnerText = assmbname 
                                    xmlHintPath.InnerText = assmbname & ".dll" 
                                    newelement.SetAttribute("Include", assmbname) 

                                    newelement.AppendChild(xmlName) 
                                    newelement.AppendChild(xmlHintPath) 

                                    xelement.AppendChild(newelement) 

                                    xmldoc.Save(projfile) 

                                    If Not attrToRemove.Contains(newelement) Then 
                                        attrToRemove.Add(newelement) 
                                    End If 
                                End If 

                            Else 
                                refinclude = String.Empty 
                            End If '// If InStr(refinclude, ",") 

                        End If '//If child.Name.Equals("Reference") 

                    Next 
                End If 
            Next 

            'If there are project references found, then lets remove them now 
            If elementToRemove.Count > 0 Then 
                ien = elementToRemove.Keys.GetEnumerator 
                While ien.MoveNext 
                    Dim _childElement As XmlElement = CType(ien.Current, XmlElement) 
                    Dim _parentElement As XmlElement = (CType(elementToRemove(ien.Current), XmlElement)) 
                    _parentElement.RemoveChild(_childElement) 
                End While 
                xmldoc.Save(projfile) 
            End If 

            'Now update all the hint paths 
            xnl = xmldoc.GetElementsByTagName("Reference") 

            For Each xelement In xnl 
                refinclude = "" 
                assmbname = "" 
                If xelement.HasChildNodes Then 

                    If xelement.HasAttribute("Include") Then 
                        refinclude = xelement.GetAttribute("Include") 
                        If InStr(refinclude, ",") Then 
                            assmbname = refinclude.Split(","c)(0) 
                        ElseIf ProjectsToBuildList.Contains(refinclude & ".dll") Then 
                            assmbname = refinclude 
                        ElseIf IsProxy(refinclude & ".dll") Then 
                            assmbname = refinclude 
                        End If 
                    End If 

                    'check if there is a <HintPath> element present 
                    Dim hintpathnode As XmlNode = Nothing 
                    For i As Integer = 0 To xelement.ChildNodes.Count - 1 
                        If String.Compare(xelement.ChildNodes(i).Name, "HintPath") = 0 Then 
                            hintpathnode = xelement.ChildNodes(i) 
                            Exit For 
                        End If 
                    Next 
                    If ((hintpathnode Is Nothing) And Not ToBeExcluded(assmbname) And assmbname.Length > 0) Then 
                        'TODO: Add an <HintPath> element 
                        hintpathnode = xmldoc.CreateElement("HintPath", "http://schemas.microsoft.com/developer/msbuild/2003") 
                        hintpathnode.InnerText = assmbname 
                        xelement.AppendChild(hintpathnode) 
                    End If 

                    Dim childNodeList As XmlNodeList = xelement.ChildNodes 

                    For Each child As XmlElement In childNodeList 

                        If child.Name.Equals("Name") Then 
                            assmbname = child.InnerText 
                        End If 
                        If child.Name.Equals("HintPath") Then 
                            hintpathAttr = child.InnerText 
                        End If 

                        If Not assmbname Is Nothing And assmbname.Length > 0 And ToBeExcluded(assmbname) And IsProxy(assmbname & ".dll") Then 
                            newHintPathAttr = buildFileLocation & assmbname & ".dll" 
                            If proxiesList.Contains(assmbname) And Not File.Exists(newHintPathAttr) Then 
                                File.Copy(CType(proxiesList(assmbname), String), newHintPathAttr, True) 
                            End If 
                            If child.Name.Equals("HintPath") Then 
                                child.InnerText = buildFileLocation & assmbname & ".dll" 
                            End If 
                            blDoSave = True 
                        End If 

                        If child.Name.Equals("HintPath") And Not assmbname Is Nothing And assmbname.Length > 0 And Not ToBeExcluded(assmbname) And Not hintpathAttr Is Nothing AndAlso hintpathAttr.Length > 0 Then 
                            newHintPathAttr = buildFileLocation & assmbname & ".dll" 
                            If Not ProjectsToBuildList.Contains(assmbname & ".dll") And Not File.Exists(newHintPathAttr) And IsProxy(assmbname & ".dll") Then 
                                If proxiesList.Contains(assmbname & ".dll") Then 
                                    File.Copy(CType(proxiesList(assmbname & ".dll"), String), newHintPathAttr, True) 
                                End If 
                            End If 
                            child.InnerText = buildFileLocation & assmbname & ".dll" 
                            blDoSave = True 
                        End If 
                    Next '// child In childNodeList 

                End If '//if element has childnodes 

            Next '//next <Reference> element 

        Else 'DOTNET_VER = 1.0 
            xnl = xmldoc.SelectNodes("//References/Reference") 

            For Each xelement In xnl 

                If xelement.HasAttribute("Name") Then 
                    name = xelement.Attributes.GetNamedItem("Name").Value 
                End If 

                'If proxyAliasList.Contains(name) Then 
                '    If xelement.HasAttribute("Guid") And _ 
                '    xelement.HasAttribute("VersionMajor") And _ 
                '    xelement.HasAttribute("VersionMinor") And _ 
                '    xelement.HasAttribute("Lcid") And _ 
                '    xelement.HasAttribute("WrapperTool") Then 

                '        xelement.RemoveAttribute("Guid") 
                '        xelement.RemoveAttribute("VersionMajor") 
                '        xelement.RemoveAttribute("VersionMinor") 
                '        xelement.RemoveAttribute("Lcid") 
                '        xelement.RemoveAttribute("WrapperTool") 

                '        xelement.SetAttribute("AssemblyName", CType(proxyAliasList(name), String)) 
                '        xelement.SetAttribute("HintPath", buildFileLocation & CType(proxyAliasList(name), String) + ".dll") 

                '        blDoSave = True 
                '    End If 
                'End If 

                'Remove project references 
                If xelement.HasAttribute("Project") Then 
                    xelement.RemoveAttribute("Project") 
                    xelement.RemoveAttribute("Package") 
                    If BuildListMapProjToAssmbName.Contains(projfile) Then 
                        xelement.SetAttribute("AssemblyName", name) 
                        xelement.SetAttribute("HintPath", buildFileLocation & name & ".dll") 
                        blDoSave = True 
                    End If 
                End If 

                If xelement.HasAttribute("AssemblyName") Then 
                    assmbname = xelement.Attributes.GetNamedItem("AssemblyName").Value 
                End If 

                If xelement.HasAttribute("HintPath") Then 
                    hintpathAttr = xelement.Attributes.GetNamedItem("HintPath").Value 
                End If 

                If ToBeExcluded(assmbname) And IsProxy(assmbname & ".dll") Then 
                    newHintPathAttr = buildFileLocation & assmbname & ".dll" 
                    If proxiesList.Contains(assmbname & ".dll") And Not File.Exists(newHintPathAttr) Then 
                        File.Copy(CType(proxiesList(assmbname & ".dll"), String), newHintPathAttr, True) 
                    End If 
                    xelement.SetAttribute("HintPath", buildFileLocation & assmbname & ".dll") 
                    blDoSave = True 
                End If 

                If Not assmbname Is Nothing And Not ToBeExcluded(assmbname) And Not hintpathAttr Is Nothing AndAlso hintpathAttr.Length > 0 Then 
                    newHintPathAttr = buildFileLocation & assmbname & ".dll" 
                    If Not ProjectsToBuildList.Contains(assmbname & ".dll") And Not File.Exists(newHintPathAttr) And IsProxy(assmbname & ".dll") Then 
                        If proxiesList.Contains(assmbname & ".dll") Then 
                            File.Copy(CType(proxiesList(assmbname & ".dll"), String), newHintPathAttr, True) 
                        End If 
                    End If 
                    xelement.SetAttribute("HintPath", buildFileLocation & assmbname & ".dll") 
                    blDoSave = True 

                End If 
            Next 
        End If '//check DOTNET_VER 

        Try 
            If blDoSave Then 
                xmldoc.Save(projfile) 
            End If 
        Catch ex As Exception 
            If dispXMLoutput Then 
                Output("<message loc=""updatehintpath""><![CDATA[ERROR while updating HintPath ! Failed to save : " + projfile + "]]> </message>") 
                Output("<message loc=""updatehintpath""><![CDATA[" + ex.Message + "]]> </message>") 
            Else 
                Output(vbCrLf + "Error while updating HintPath ! Failed to save : " + projfile + vbCrLf + ex.Message, ConsoleTools.ConsoleColor.red) 
            End If 
            WriteLog(vbCrLf + "Error while updating HintPath ! Failed to save : " + projfile + vbCrLf + ex.Message) 
            blUpdateHintPathOk = False 
            Throw ex 
        Finally 
            xmldoc = Nothing 
        End Try 

        File.SetAttributes(projfile, FileAttributes.ReadOnly) 

        Return blUpdateHintPathOk 
    End Function 

According to Visual Studio Code Metrics the Maintainability Index of UpdateHintPath is 16. This is not good.

The first thing you should do before undertaking any refactoring is make sure the code is covered by some automated unit test.  If it isn't then you should apply one or more of the techniques outlined in Michael Feathers awesome book Working Effectively with Legacy Code. Once the code to be refactored is under test and all tests are green then we can proceed.

 For the Extract Method refactoring with CodeRush Xpress we simply highlight the code we want to extract to it's own method and press the ctrl+` key. In this case we will highlight the first part of the big "If" statement:
If GetDotNetVersion(projfile) = DOTNETVERSION.TWO Then
Hit ctrl+` and we get:
Coderush Refactor Popup Menu

Then make sure Extract Method is selected and press Enter.

Extract Method Placement

Next use the Up and Down arrows to move the red line to where you want the new method code to be put and hit Enter.


CodeRush takes a guess at the extracted method name based on a nearby comment which can be bang on sometimes and all you would do is hit Enter and be done, but in this case I would like to change it to something that better describes what the new method is doing UpdateHintPathForDotNetTwo.

Change Method Name

After changing the name and hitting Enter the new method creation is completed and you will notice that Coderush also took a best guess as to what needs to be passed into the new method, more on this later. To get back to the original method all that needs to be done is press the Esc key.

Back in the original method

And there we can see our new method call we just created. Then I repeated the process for the Else statement and called it UpdateHintPathForDotNetOne.

Refactor Else statement

Now we have quite a few parameters that are being passed in unnecessarily to the new methods so we clean those up and here is the final refactored method:
    Private Function UpdateHintPath(ByVal projfile As String) As Boolean 

        Dim xmldoc As XmlDocument 
        Dim blDoSave As Boolean = False 
        Dim blUpdateHintPathOk As Boolean = True 
         
        File.SetAttributes(projfile, FileAttributes.Normal) 

        xmldoc = New XmlDocument 
        xmldoc.Load(projfile) 

        If GetDotNetVersion(projfile) = DOTNETVERSION.TWO Then 
            UpdateHintPathForDotNetTwo(projfile, xmldoc, blDoSave) 
        Else 'DOTNET_VER = 1.0 
            UpdateHintPathForDotNetOne(projfile, xmldoc, blDoSave) 
        End If '//check DOTNET_VER 

        Try 
            If blDoSave Then 
                xmldoc.Save(projfile) 
            End If 
        Catch ex As Exception 
            If dispXMLoutput Then 
                Output("<message loc=""updatehintpath""><![CDATA[ERROR while updating HintPath ! Failed to save : " + projfile + "]]> </message>") 
                Output("<message loc=""updatehintpath""><![CDATA[" + ex.Message + "]]> </message>") 
            Else 
                Output(vbCrLf + "Error while updating HintPath ! Failed to save : " + projfile + vbCrLf + ex.Message, ConsoleTools.ConsoleColor.red) 
            End If 
            WriteLog(vbCrLf + "Error while updating HintPath ! Failed to save : " + projfile + vbCrLf + ex.Message) 
            blUpdateHintPathOk = False 
            Throw ex 
        Finally 
            xmldoc = Nothing 
        End Try 

        File.SetAttributes(projfile, FileAttributes.ReadOnly) 

        Return blUpdateHintPathOk 
    End Function 

Visual Studio Code Metrics is now reporting that the Maintainability Index of the new UpdateHintPath is at a respectable 50. Finally remember to run the unit tests to make sure they are still all green.

Comments

Popular posts from this blog

ToMethodObject Refactoring

How to unlock a file in Team Foundation Server(TFS)

Show/Hide formatting text in MS Word 2010