HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Extract Interface

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
interfaceextractstackoverflow

Problem

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 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 _declarations;
public IEnumerable 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 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 =>

Solution

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.

Code Snippets

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 readonly string[] PrimitiveTypes =

Context

StackExchange Code Review Q#115089, answer score: 3

Revisions (0)

No revisions yet.