你升职了，你升职了，我们都升职了！​

Question

我最近对Rubber鸭进行的重构称为Introduce，它将局部变量提升到私有字段。

三个重写的Refactor()方法是IRefactoring的成员，用于启动重构序列。其他方法是worker方法。

总的来说，我对此很满意，但是有一些事情困扰着我。首先，Refactor(Declaration)对声明类型有严格的要求，但接受任何声明，而不管声明类型如何。除了投掷之外，还有什么办法可以强制执行吗？我应该在这里扔吗？我应该直接回来吗？

RemoveVariable()和RemoveExtraComma()看起来都有点笨重。有更干净的方法吗？

public class IntroduceField : IRefactoring
{
    private readonly IList<Declaration> _declarations;
    private readonly IActiveCodePaneEditor _editor;
    private Declaration _targetDeclaration;
    private readonly IMessageBox _messageBox;

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

    public void Refactor()
    {
        if (_targetDeclaration == null)
        {
            _messageBox.Show(RubberduckUI.PromoteVariable_InvalidSelection);
            return;
        }

        RemoveVariable();
        AddField();
    }

    public void Refactor(QualifiedSelection selection)
    {
        _targetDeclaration = FindSelection(selection);
        Refactor();
    }

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

        _targetDeclaration = target;
        Refactor();
    }

    private void AddField()
    {
        var module = _targetDeclaration.QualifiedName.QualifiedModuleName.Component.CodeModule;
        module.InsertLines(module.CountOfDeclarationLines + 1, GetFieldDefinition());
    }

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

        var variableStmtContext = GetVariableStmtContext(_targetDeclaration);

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

        var oldLines = _editor.GetLines(selection);

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

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

        _editor.DeleteLines(selection);
        _editor.InsertLines(selection.StartLine, newLines);
    }

    private Selection GetVariableStmtContextSelection(Declaration target)
    {
        var statement = GetVariableStmtContext(target);

        return new Selection(statement.Start.Line, statement.Start.Column,
                statement.Stop.Line, statement.Stop.Column);
    }

    private VBAParser.VariableStmtContext GetVariableStmtContext(Declaration target)
    {
        var statement = target.Context.Parent.Parent as VBAParser.VariableStmtContext;
        if (statement == null)
        {
            throw new MissingMemberException("Statement not found");
        }

        return statement;
    }

    private string RemoveExtraComma(string str)
    {
        if (str.Count(c => c == ',') == 1)
        {
            return str.Remove(str.IndexOf(','), 1);
        }

        var significantCharacterAfterComma = false;

        for (var index = 0; index < str.Length; index++)
        {
            if (!char.IsWhiteSpace(str[index]) && str[index] != '_' && str[index] != ',')
            {
                significantCharacterAfterComma = true;
            }
            if (str[index] == ',')
            {
                significantCharacterAfterComma = false;
            }

            if (!significantCharacterAfterComma && str[index] == ',')
            {
                return str.Remove(index, 1);
            }
        }

        return str;
    }

    private bool HasMultipleDeclarationsInStatement(Declaration target)
    {
        var statement = target.Context.Parent as VBAParser.VariableListStmtContext;

        if (statement == null) { return false; }

        return statement.children.Count(i => i is VBAParser.VariableSubStmtContext) > 1;
    }

    private string GetFieldDefinition()
    {
        if (_targetDeclaration == null) { return null; }

        return "Private " + _targetDeclaration.IdentifierName + " As " + _targetDeclaration.AsTypeName;
    }

    private Declaration FindSelection(QualifiedSelection selection)
    {
        var target = _declarations
            .Where(item => !item.IsBuiltIn)
            .FirstOrDefault(item => item.IsSelected(selection) && item.DeclarationType == DeclarationType.Variable
                                 || item.References.Any(r => r.IsSelected(selection) &&
                                    r.Declaration.DeclarationType == DeclarationType.Variable));

        if (target != null) { return target; }

        var targets = _declarations
            .Where(item => !item.IsBuiltIn
                           && item.ComponentName == selection.QualifiedName.ComponentName
                           && item.DeclarationType == DeclarationType.Variable);

        foreach (var declaration in targets)
        {
            var declarationSelection = new Selection(declaration.Context.Start.Line,
                                                declaration.Context.Start.Column,
                                                declaration.Context.Stop.Line,
                                                declaration.Context.Stop.Column + declaration.Context.Stop.Text.Length);

            if (declarationSelection.Contains(selection.Selection) ||
                !HasMultipleDeclarationsInStatement(declaration) && GetVariableStmtContextSelection(declaration).Contains(selection.Selection))
            {
                return declaration;
            }

            var reference =
                declaration.References.FirstOrDefault(r => r.Selection.Contains(selection.Selection));

            if (reference != null)
            {
                return reference.Declaration;
            }
        }
        return null;
    }
}

Answer 1

公共IntroduceField(RubberduckParserState parseResult，IActiveCodePaneEditor编辑器，IMessageBox messageBox) { _declarations = parseResult.AllDeclarations.ToList()；_editor =编辑器；_messageBox = messageBox；}

为什么不在这里传递一个IEnumerable<Declaration>而不是一个RubberduckParserState对象呢？现在，IntroduceField(顺便说一句，类名看起来很奇怪)耦合到该对象和Declaration。

滥用var

在RemoveVariable()方法中，我看不到var类型的任何正确用法。只有在任务的右边明显显示类型应该是什么的情况下，才应该使用var。在其他方法中，这种情况也在发生，但没有发生那么多。

RemoveExtraComma()

这个方法看起来很奇怪。让我们检查一下它正在做什么：

1. 如果只有一个逗号，它将删除该逗号并返回结果。
2. 对字符串的字符执行奇怪的循环，删除逗号的第一次出现并返回结果。
3. 如果没有找到逗号，则返回原始参数。

如果这不是一个bug，那么这可以简化为

private static string RemoveExtraComma(string str)
{
    int commaPosition = str.IndexOf(',');
    if (commaPosition >= 0)
    {
        return str.Remove(commaPosition, 1);
    }

    return str;
}

Answer 2

公共空重构(){ if (_targetDeclaration == null) {_targetDeclaration返回；} RemoveVariable()；AddField()；}

不确定除了Rubber鸭子的架构师以外是否还有其他人能指出这一个，但这感觉不太正确。void Refactor()的无参数过载将是“智能”重载，它可以计算出所有的事情。

当调用该方法时，您的实现假设已经有一个_targetDeclaration，但是它确实应该从当前的代码窗格选择中计算出来，然后计算出是否显示“无效选择”消息。

私人声明_targetDeclaration；

唯一不是readonly的字段确实很突出，让我怀疑是否会有一种方法来避免使用该字段--而且，如果您使无参数重载的工作方式有所不同的话。

我的意思是，反转工作流-让无参数的一个计算出target并将它传递给作为参数的重载-然后让重载调用RemoveVariable和AddField。

公开无效重构(声明目标){ if (target.DeclarationType != DeclarationType.Variable) {引发新ArgumentException(“无效声明类型”)；}

我认为这里有一个异常是可以的；这个重载将由Rubber鸭代码调用，如果用任何非变量的target调用它，就会出现编程错误--抛出异常是正确的。

RemoveVariable和AddField之间存在时态耦合:如果首先调用AddField，则RemoveVariable中断，因为代码模块中的行现在被偏移。我不认为有什么办法可以避免这种情况，只是把它说得很清楚：

private void PromoteVariable(Declaration target)
{
    // must remove variable BEFORE adding the field
    RemoveVariable(target);
    AddField(target);
}

注意，我将target作为参数传递，而不是将其保持在实例级别。

最后一件事：RubberduckParserState parseResult构造函数参数不再是真正的“解析结果”(相对于较早版本的代码库，解析器返回一个实际的结果)--更好的名称可以是parserState，或者简单地说是state。

Answer 3

例外

抛出新的ArgumentException(“无效声明类型”)；

即使这个方法中只有一个参数，我也应该提供参数名，这样就不需要查看人员调试了。这是在我正在使用的.NET 4.5中完成的，如：

throw new ArgumentException("Invalid declaration type", "target");

作为参考，C# 6.0有一个名为nameof(target)的nameof()表达式，并返回名称。这是更好的选择，因为它直接引用变量，如果变量名被更改，则强制它进行更新。

Linq简化

每次查询_declarations列表时，我都会检查唯一返回的声明是否内置(!d.IsBuiltIn)并具有DeclarationType.Variable类型。我可以通过选择构造函数中的可读性和性能来提高可读性和性能(注意，我更改了Mat's Mug建议的名称)：

_declarations =
    parserState.AllDeclarations.Where(i => !i.IsBuiltIn && i.DeclarationType == DeclarationType.Variable)
        .ToList();