Move Closer To Usage

One of my latest refactorings for Rubberduck is Move Closer To Usage. The refactoring will take a field and move it just above the reference to it only if it is used in a single method, or take a variable declaration and move it just above its first call.

As promised, I moved many of the duplicated support methods into extension methods, although I have a few left that are duplicated between a few refactorings. Also, thanks to Thomas Eyde, I found some bugs in RemoveComma(), which are now fixed.

Additionally, this refactoring does not have a user interface, so there is just one file. Be sure to let me know what you think!

public class MoveCloserToUsageRefactoring : IRefactoring
{
    private readonly List<Declaration> _declarations;
    private readonly IActiveCodePaneEditor _editor;
    private readonly IMessageBox _messageBox;

    public MoveCloserToUsageRefactoring(RubberduckParserState parseResult, IActiveCodePaneEditor editor, IMessageBox messageBox)
    {
        _declarations = parseResult.AllDeclarations.ToList();
        _editor = editor;
        _messageBox = messageBox;
    }

    public void Refactor()
    {
        var qualifiedSelection = _editor.GetSelection();
        if (qualifiedSelection != null)
        {
            Refactor(_declarations.FindVariable(qualifiedSelection.Value));
        }
        else
        {
            _messageBox.Show("Invalid Selection.", "Rubberduck - Move Closer To Usage", System.Windows.Forms.MessageBoxButtons.OK,
                System.Windows.Forms.MessageBoxIcon.Exclamation);
        }
    }

    public void Refactor(QualifiedSelection selection)
    {
        Refactor(_declarations.FindVariable(selection));
    }

    public void Refactor(Declaration target)
    {
        if (target.DeclarationType != DeclarationType.Variable)
        {
            throw new ArgumentException(@"Invalid Argument", "target");
        }

        if (!target.References.Any())
        {
            var message = string.Format(RubberduckUI.MoveCloserToUsage_TargetHasNoReferences, target.IdentifierName);

            _messageBox.Show(message, RubberduckUI.MoveCloserToUsage_Caption, System.Windows.Forms.MessageBoxButtons.OK,
                System.Windows.Forms.MessageBoxIcon.Exclamation);

            return;
        }

        if (TargetIsReferencedFromMultipleMethods(target))
        {
            var message = string.Format(RubberduckUI.MoveCloserToUsage_TargetIsUsedInMultipleMethods, target.IdentifierName);
            _messageBox.Show(message, RubberduckUI.MoveCloserToUsage_Caption, System.Windows.Forms.MessageBoxButtons.OK,
                System.Windows.Forms.MessageBoxIcon.Exclamation);

            return;
        }

        MoveDeclaration(target);
    }

    private bool TargetIsReferencedFromMultipleMethods(Declaration target)
    {
        var firstReference = target.References.FirstOrDefault();

        return firstReference != null && target.References.Any(r => r.ParentScope != firstReference.ParentScope);
    }

    private void MoveDeclaration(Declaration target)
    {
        InsertDeclaration(target);
        RemoveVariable(target);
    }

    private void InsertDeclaration(Declaration target)
    {
        var firstReference = target.References.OrderBy(r => r.Selection.StartLine).First();

        var oldLines = _editor.GetLines(firstReference.Selection);
        var newLines = oldLines.Insert(firstReference.Selection.StartColumn - 1, GetDeclarationString(target));

        _editor.DeleteLines(firstReference.Selection);
        _editor.InsertLines(firstReference.Selection.StartLine, newLines);
    }

    private string GetDeclarationString(Declaration target)
    {
        return Environment.NewLine + "    Dim " + target.IdentifierName + " As " + target.AsTypeName + Environment.NewLine;
    }

    private void RemoveVariable(Declaration target)
    {
        Selection selection;
        var declarationText = target.Context.GetText();
        var multipleDeclarations = target.HasMultipleDeclarationsInStatement();

        var variableStmtContext = target.GetVariableStmtContext();

        if (!multipleDeclarations)
        {
            declarationText = variableStmtContext.GetText();
            selection = target.GetVariableStmtContextSelection();
        }
        else
        {
            selection = new Selection(target.Context.Start.Line, target.Context.Start.Column,
                target.Context.Stop.Line, target.Context.Stop.Column);
        }

        var oldLines = _editor.GetLines(selection);

        var newLines = oldLines.Replace(" _" + Environment.NewLine, string.Empty)
            .Remove(selection.StartColumn, declarationText.Length);

        if (multipleDeclarations)
        {
            selection = target.GetVariableStmtContextSelection();
            newLines = RemoveExtraComma(_editor.GetLines(selection).Replace(oldLines, newLines),
                target.CountOfDeclarationsInStatement(), target.IndexOfVariableDeclarationInStatement());
        }

        _editor.DeleteLines(selection);

        if (newLines.Trim() != string.Empty)
        {
            _editor.InsertLines(selection.StartLine, newLines);
        }
    }

    private string RemoveExtraComma(string str, int numParams, int indexRemoved)
    {
        // Example use cases for this method (fields and variables):
        // Dim fizz as Boolean, dizz as Double
        // Private fizz as Boolean, dizz as Double
        // Public fizz as Boolean, _
        //        dizz as Double
        // Private fizz as Boolean _
        //         , dizz as Double _
        //         , iizz as Integer

        // Before this method is called, the parameter to be removed has 
        // already been removed.  This means 'str' will look like:
        // Dim fizz as Boolean, 
        // Private , dizz as Double
        // Public fizz as Boolean, _
        //        
        // Private  _
        //         , dizz as Double _
        //         , iizz as Integer

        // This method is responsible for removing the redundant comma
        // and returning a string similar to:
        // Dim fizz as Boolean
        // Private dizz as Double
        // Public fizz as Boolean _
        //        
        // Private  _
        //          dizz as Double _
        //         , iizz as Integer

        var commaToRemove = numParams == indexRemoved ? indexRemoved - 1 : indexRemoved;

        return str.Remove(str.NthIndexOf(',', commaToRemove), 1);
    }
}

There’s not a lot to be said in my opinion. You’ve done a good job of using variables where they help clarify the intent and everything seems to be well named. The code is easy to read, even if some lines get kind of long and could be split across several lines. (Linq has a way of going sideways instead of down if you let it.)

There is this here.

throw new ArgumentException(@"Invalid Argument", "target")

I copied it in isolation to simulate what would happen if this gets thrown. Can you tell what exactly went wrong? No? Maybe some context would help.

    if (target.DeclarationType != DeclarationType.Variable)
    {
        throw new ArgumentException(@"Invalid Argument", "target");

Crystal clear, but we had to look at the code to know it. I’d recommend a better message.

    if (target.DeclarationType != DeclarationType.Variable)
    {
        throw new ArgumentException("Invalid Argument. DeclarationType must be a Variable", "target");

Also note that there wasn’t a need for the string literal identifier (@).

There’s also this.

    {
        _messageBox.Show("Invalid Selection.", "Rubberduck - Move Closer To Usage", System.Windows.Forms.MessageBoxButtons.OK,
            System.Windows.Forms.MessageBoxIcon.Exclamation);
    }

• Those strings should be in a resource file so they can be localized. Rubberduck has been translated into several languages at this point.

• I’m not entirely sure why you fully qualified the MessageBox enums. If you don’t want to import the Forms namespace, I understand. However, you could alias the enums that you need to interact with the IMessageBox abstraction.

   using MessageBoxButtons = System.Windows.Forms.MessageBoxButtons;
   using MessageBoxIcons = System.Windows.Forms.MessageBoxIcons;