Extract Interface

Question

One of the latest refactorings for Rubberduck is Extract Interface. This refactoring will take a class, display all public members, and allow you to select which members you wish to include in your interface. Next, it creates the interface, adds Implement <Interface Name> to the top of the file, and calls Implement Interface to implement empty members of the interface. Unfortunately, due to the time to parse and resolve references, we chose to not rename the existing members.

This is the model for the refactoring:

public class ExtractInterfaceModel
{
    private readonly RubberduckParserState _parseResult;
    public RubberduckParserState ParseResult { get { return _parseResult; } }

    private readonly IEnumerable<Declaration> _declarations;
    public IEnumerable<Declaration> Declarations { get { return _declarations; } }

    private readonly QualifiedSelection _selection;
    public QualifiedSelection Selection { get { return _selection; } }

    private readonly Declaration _targetDeclaration;
    public Declaration TargetDeclaration { get { return _targetDeclaration; } }

    public string InterfaceName { get; set; }
    public List<InterfaceMember> Members { get; set; }

    private static readonly DeclarationType[] DeclarationTypes =
    {
        DeclarationType.Class,
        DeclarationType.Document,
        DeclarationType.UserForm
    };

    public readonly string[] PrimitiveTypes =
    {
        Tokens.Boolean,
        Tokens.Byte,
        Tokens.Date,
        Tokens.Decimal,
        Tokens.Double,
        Tokens.Long,
        Tokens.LongLong,
        Tokens.LongPtr,
        Tokens.Integer,
        Tokens.Single,
        Tokens.String,
        Tokens.StrPtr
    };

    public ExtractInterfaceModel(RubberduckParserState parseResult, QualifiedSelection selection)
    {
        _parseResult = parseResult;
        _selection = selection;
        _declarations = parseResult.AllDeclarations.ToList();

        _targetDeclaration =
            _declarations.SingleOrDefault(
                item =>
                    !item.IsBuiltIn && DeclarationTypes.Contains(item.DeclarationType)
                    && item.Project == selection.QualifiedName.Project
                    && item.QualifiedSelection.QualifiedName == selection.QualifiedName);

        InterfaceName = "I" + TargetDeclaration.IdentifierName;

         Members = _declarations.Where(item => !item.IsBuiltIn &&
                                            item.Project == _targetDeclaration.Project &&
                                            item.ComponentName == _targetDeclaration.ComponentName &&
                                            item.Accessibility == Accessibility.Public &&
                                            item.DeclarationType != DeclarationType.Variable &&
                                            item.DeclarationType != DeclarationType.Event)
                                 .OrderBy(o => o.Selection.StartLine)
                                 .ThenBy(t => t.Selection.StartColumn)
                                 .Select(d => new InterfaceMember(d, _declarations))
                                 .ToList();
    }
}

And the presenter:

public interface IExtractInterfacePresenter
{
    ExtractInterfaceModel Show();
}

public class ExtractInterfacePresenter : IExtractInterfacePresenter
{
    private readonly IExtractInterfaceView _view;
    private readonly ExtractInterfaceModel _model;

    public ExtractInterfacePresenter(IExtractInterfaceView view, ExtractInterfaceModel model)
    {
        _view = view;
        _model = model;
    }

    public ExtractInterfaceModel Show()
    {
        if (_model.TargetDeclaration == null) { return null; }

        _view.ComponentNames =
            _model.TargetDeclaration.Project.VBComponents.Cast<VBComponent>().Select(c => c.Name).ToList();
        _view.InterfaceName = _model.InterfaceName;
        _view.Members = _model.Members;

        if (_view.ShowDialog() != DialogResult.OK)
        {
            return null;
        }

        _model.InterfaceName = _view.InterfaceName;
        _model.Members = _view.Members;
        return _model;
    }
}

Next comes the refactoring:

public class ExtractInterfaceRefactoring : IRefactoring
{
    private readonly RubberduckParserState _state;
    private readonly IRefactoringPresenterFactory<ExtractInterfacePresenter> _factory;
    private readonly IActiveCodePaneEditor _editor;
    private ExtractInterfaceModel _model;

    public ExtractInterfaceRefactoring(RubberduckParserState state, IRefactoringPresenterFactory<ExtractInterfacePresenter> factory,
        IActiveCodePaneEditor editor)
    {
        _state = state;
        _factory = factory;
        _editor = editor;
    }

    public void Refactor()
    {
        var presenter = _factory.Create();
        if (presenter == null)
        {
            return;
        }

        _model = presenter.Show();

        if (_model == null) { return; }

        AddInterface();
    }

    public void Refactor(QualifiedSelection target)
    {
        _editor.SetSelection(target);
        Refactor();
    }

    public void Refactor(Declaration target)
    {
        _editor.SetSelection(target.QualifiedSelection);
        Refactor();
    }

    private void AddInterface()
    {
        var interfaceComponent = _model.TargetDeclaration.Project.VBComponents.Add(vbext_ComponentType.vbext_ct_ClassModule);
        interfaceComponent.Name = _model.InterfaceName;

        _editor.InsertLines(1, GetInterface());

        var module = _model.TargetDeclaration.QualifiedSelection.QualifiedName.Component.CodeModule;

        var implementsLine = module.CountOfDeclarationLines + 1;
        module.InsertLines(implementsLine, "Implements " + _model.InterfaceName);

        _state.RequestParse(ParserState.Ready);
        var qualifiedSelection = new QualifiedSelection(_model.TargetDeclaration.QualifiedSelection.QualifiedName,
            new Selection(implementsLine, 1, implementsLine, 1));

        var implementInterfaceRefactoring = new ImplementInterfaceRefactoring(_state, _editor, new MessageBox());
        implementInterfaceRefactoring.Refactor(qualifiedSelection);
    }

    private string GetInterface()
    {
        return "Option Explicit" + Environment.NewLine + string.Join(Environment.NewLine, _model.Members.Where(m => m.IsSelected));
    }
}

And the support classes, InterfaceMember and Parameter:

public class Parameter
{
    public string ParamAccessibility { get; set; }
    public string ParamName { get; set; }
    public string ParamType { get; set; }

    public override string ToString()
    {
        return ParamAccessibility + " " + ParamName + " As " + ParamType;
    }
}

public class InterfaceMember
{
    public Declaration Member { get; set; }
    public IEnumerable<Parameter> MemberParams { get; set; }
    public string Type { get; set; }

    public string MemberType { get; set; }
    public string PropertyType { get; set; }

    public bool IsSelected { get; set; }
    public string MemberSignature
    {
        get
        {
            var signature = Member.IdentifierName + "(" +
                string.Join(", ", MemberParams.Select(m => m.ParamType)) + ")";

            return Type == null ? signature : signature + " As " + Type;
        }
    }

    public string FullMemberSignature
    {
        get
        {
            var signature = Member.IdentifierName + "(" +
                string.Join(", ", MemberParams) + ")";

            return Type == null ? signature : signature + " As " + Type;
        }
    }

    public InterfaceMember(Declaration member, IEnumerable<Declaration> declarations)
    {
        Member = member;
        Type = member.AsTypeName;

        GetMethodType();

        MemberParams = declarations.Where(item => item.DeclarationType == DeclarationType.Parameter &&
                                      item.ParentScope == Member.Scope)
                                   .OrderBy(o => o.Selection.StartLine)
                                   .ThenBy(t => t.Selection.StartColumn)
                                   .Select(p => new Parameter
                                   {
                                       ParamAccessibility = ((VBAParser.ArgContext)p.Context).BYREF() == null ? Tokens.ByVal : Tokens.ByRef,
                                       ParamName = p.IdentifierName,
                                       ParamType = p.AsTypeName
                                   })
                                   .ToList();

        if (PropertyType == "Get")
        {
            MemberParams = MemberParams.Take(MemberParams.Count() - 1);
        }


        IsSelected = false;
    }

    private void GetMethodType()
    {
        var context = Member.Context;

        var subStmtContext = context as VBAParser.SubStmtContext;
        if (subStmtContext != null)
        {
            MemberType = Tokens.Sub;
        }

        var functionStmtContext = context as VBAParser.FunctionStmtContext;
        if (functionStmtContext != null)
        {
            MemberType = Tokens.Function;
        }

        var propertyGetStmtContext = context as VBAParser.PropertyGetStmtContext;
        if (propertyGetStmtContext != null)
        {
            MemberType = Tokens.Property;
            PropertyType = Tokens.Get;
        }

        var propertyLetStmtContext = context as VBAParser.PropertyLetStmtContext;
        if (propertyLetStmtContext != null)
        {
            MemberType = Tokens.Property;
            PropertyType = Tokens.Let;
        }

        var propertySetStmtContext = context as VBAParser.PropertySetStmtContext;
        if (propertySetStmtContext != null)
        {
            MemberType = Tokens.Property;
            PropertyType = Tokens.Set;
        }
    }

    public override string ToString()
    {
        return "Public " + MemberType + " " + PropertyType + " " + FullMemberSignature + Environment.NewLine + "End " + MemberType +
               Environment.NewLine;
    }
}

This is the dialog code behind:

public partial class ExtractInterfaceDialog : Form, IExtractInterfaceView
{
    public string InterfaceName
    {
        get { return InterfaceNameBox.Text; }
        set { InterfaceNameBox.Text = value; }
    }

    private List<InterfaceMember> _members;
    public List<InterfaceMember> Members
    {
        get { return _members; }
        set
        {
            _members = value;
            InitializeParameterGrid();
        }
    }

    public List<string> ComponentNames { get; set; }

    public ExtractInterfaceDialog()
    {
        InitializeComponent();

        InterfaceNameBox.TextChanged += InterfaceNameBox_TextChanged;
        InterfaceMembersGridView.CellValueChanged += InterfaceMembersGridView_CellValueChanged;
        SelectAllButton.Click += SelectAllButton_Click;
        DeselectAllButton.Click += DeselectAllButton_Click;
    }

    void InterfaceNameBox_TextChanged(object sender, EventArgs e)
    {
        ValidateNewName();
    }

    void InterfaceMembersGridView_CellValueChanged(object sender, DataGridViewCellEventArgs e)
    {
        _members.ElementAt(e.RowIndex).IsSelected =
            (bool) InterfaceMembersGridView.Rows[e.RowIndex].Cells[e.ColumnIndex].Value;
    }

    void SelectAllButton_Click(object sender, EventArgs e)
    {
        ToggleSelection(true);
    }

    void DeselectAllButton_Click(object sender, EventArgs e)
    {
        ToggleSelection(false);
    }

    private void InitializeParameterGrid()
    {
        InterfaceMembersGridView.AutoGenerateColumns = false;
        InterfaceMembersGridView.Columns.Clear();
        InterfaceMembersGridView.DataSource = Members;
        InterfaceMembersGridView.AlternatingRowsDefaultCellStyle.BackColor = Color.Lavender;
        InterfaceMembersGridView.MultiSelect = false;

        var isSelected = new DataGridViewCheckBoxColumn
        {
            AutoSizeMode = DataGridViewAutoSizeColumnMode.AllCells,
            Name = "IsSelected",
            DataPropertyName = "IsSelected",
            HeaderText = string.Empty,
            ReadOnly = false
        };

        var signature = new DataGridViewTextBoxColumn
        {
            AutoSizeMode = DataGridViewAutoSizeColumnMode.Fill,
            Name = "Members",
            DataPropertyName = "MemberSignature",
            ReadOnly = true
        };

        InterfaceMembersGridView.Columns.AddRange(isSelected, signature);
    }

    void ToggleSelection(bool state)
    {
        foreach (var row in InterfaceMembersGridView.Rows.Cast<DataGridViewRow>())
        {
            row.Cells["IsSelected"].Value = state;
        }
    }

    private void ValidateNewName()
    {
        var tokenValues = typeof(Tokens).GetFields().Select(item => item.GetValue(null)).Cast<string>().Select(item => item);

        OkButton.Enabled = !ComponentNames.Contains(InterfaceName)
                           && char.IsLetter(InterfaceName.FirstOrDefault())
                           && !tokenValues.Contains(InterfaceName, StringComparer.InvariantCultureIgnoreCase)
                           && !InterfaceName.Any(c => !char.IsLetterOrDigit(c) && c != '_');

        InvalidNameValidationIcon.Visible = !OkButton.Enabled;
    }
}

Please tell me everything that can be improved with this code. The more non-trivial improvements you make, the better. Nitpicks are also welcome.

Answer 1

This section of code feels funny to me.

private static readonly DeclarationType[] DeclarationTypes =
{
    DeclarationType.Class,
    DeclarationType.Document,
    DeclarationType.UserForm
};

public readonly string[] PrimitiveTypes =
{
    Tokens.Boolean,
    Tokens.Byte,
    Tokens.Date,
    Tokens.Decimal,
    Tokens.Double,
    Tokens.Long,
    Tokens.LongLong,
    Tokens.LongPtr,
    Tokens.Integer,
    Tokens.Single,
    Tokens.String,
    Tokens.StrPtr
}

These really feel like they belong to another class. I'm not really sure what to name that class, but these belong closer to the Parser. Maybe they're static members of DeclarationTypes and Tokens respectively. Maybe they're extension methods, or maybe they both belong to some helper class, but I have a hard time believing that no other IRefactorings need to know which tokens are primitive types, or which declaration types are classes.

The other issue with these is that the arrays can be modified, if someone was silly enough to do it.

public readonly string[] PrimitiveTypes =

The readonly modifier only means that we can't assign the identifier a different reference. Nothing stops us from modifying the internals of the array. I'd reach for a ReadOnlyCollection of some kind. ReadOnlyCollection is designed to be a base class, so this gives us one more reason to extract these useful snippets into some bit of reusable code.