snippetcsharpMinor
Implement Interface Refactoring
Viewed 0 times
refactoringinterfaceimplement
Problem
The latest refactoring for Rubberduck is Implement Interface. This refactoring will check whether the active class fully implements the selected interface by comparing the definitions. It knows the class based on the
This extension method finds the interface based on the statement, which is a reference to the interface class. The
This is the refactoring itself. My main concern here is how I alternate between using the
Implements statement, which looks like Implements IXyz.This extension method finds the interface based on the statement, which is a reference to the interface class. The
implementsStmt contains the 'Implements' keyword, and the reference variable contains the name of the interface. I need to combine the two selections for user selections such as "Impl[ements IX]yz", with the braces showing the beginning and end of the selection. I return null if a statement is not found that contains the selection[SuppressMessage("ReSharper", "LoopCanBeConvertedToQuery")]
public static Declaration FindInterface(this IEnumerable declarations, QualifiedSelection selection)
{
foreach (var declaration in declarations.FindInterfaces())
{
foreach (var reference in declaration.References)
{
var implementsStmt = reference.Context.Parent as VBAParser.ImplementsStmtContext;
if (implementsStmt == null) { continue; }
var completeSelection = new Selection(implementsStmt.GetSelection().StartLine,
implementsStmt.GetSelection().StartColumn, reference.Selection.EndLine,
reference.Selection.EndColumn);
if (reference.QualifiedModuleName.ComponentName == selection.QualifiedName.ComponentName &&
reference.QualifiedModuleName.Project == selection.QualifiedName.Project &&
completeSelection.Contains(selection.Selection))
{
return declaration;
}
}
}
return null;
}This is the refactoring itself. My main concern here is how I alternate between using the
Tokens class and manually typing out the string. I prefer using the Tokens class to prevent typos, &c., but I use a Solution
FindInterface()There is no need to create the
Selection completeSelection if the ComponentName or the Project don't match. So better create the completeSelection inside the if and add another if condition for the check if completeSelection.Contains(selection.Selection) like so [SuppressMessage("ReSharper", "LoopCanBeConvertedToQuery")]
public static Declaration FindInterface(this IEnumerable declarations, QualifiedSelection selection)
{
foreach (var declaration in declarations.FindInterfaces())
{
foreach (var reference in declaration.References)
{
var implementsStmt = reference.Context.Parent as VBAParser.ImplementsStmtContext;
if (implementsStmt == null) { continue; }
if (reference.QualifiedModuleName.ComponentName == selection.QualifiedName.ComponentName &&
reference.QualifiedModuleName.Project == selection.QualifiedName.Project)
{
var completeSelection = new Selection(implementsStmt.GetSelection().StartLine,
implementsStmt.GetSelection().StartColumn, reference.Selection.EndLine,
reference.Selection.EndColumn);
if (completeSelection.Contains(selection.Selection))
{
return declaration;
}
}
}
}
return null;
}ImplementInterfaceRefactoring In the
Refactor() method it isn't obvious that the returned value from the call to GetSelection() is a nullable QualifiedSelection so the usage of the var type isn't good. IMO the class shouldn't interact with the UI by showing some messagebox. Why don't you return a state from the
Refactor() methods indicating failure/success ?In that way it could be tested much better.
GetInterfaceMember() Instead of the
switch statement I would like to suggest using a Dictionary>. That would reduce the method to something like this
private void InitializeMemberDictionary()
{
dict = new Dictionary>();
dict.Add("Sub", SubStmt(member));
dict.Add(.....
....
....
....
}
private Dictionary> dict;
private string GetInterfaceMember(Declaration member)
{
string memberType = GetMemberType(member);
Func func;
if (!dict.TryGetValue(memberType, out func))
{
return string.Empty;
}
return func(member);
}Code Snippets
[SuppressMessage("ReSharper", "LoopCanBeConvertedToQuery")]
public static Declaration FindInterface(this IEnumerable<Declaration> declarations, QualifiedSelection selection)
{
foreach (var declaration in declarations.FindInterfaces())
{
foreach (var reference in declaration.References)
{
var implementsStmt = reference.Context.Parent as VBAParser.ImplementsStmtContext;
if (implementsStmt == null) { continue; }
if (reference.QualifiedModuleName.ComponentName == selection.QualifiedName.ComponentName &&
reference.QualifiedModuleName.Project == selection.QualifiedName.Project)
{
var completeSelection = new Selection(implementsStmt.GetSelection().StartLine,
implementsStmt.GetSelection().StartColumn, reference.Selection.EndLine,
reference.Selection.EndColumn);
if (completeSelection.Contains(selection.Selection))
{
return declaration;
}
}
}
}
return null;
}private void InitializeMemberDictionary()
{
dict = new Dictionary<string, Func<string, Declaration>>();
dict.Add("Sub", SubStmt(member));
dict.Add(.....
....
....
....
}
private Dictionary<string, Func<string, Declaration>> dict;
private string GetInterfaceMember(Declaration member)
{
string memberType = GetMemberType(member);
Func<string, Declaration> func;
if (!dict.TryGetValue(memberType, out func))
{
return string.Empty;
}
return func(member);
}Context
StackExchange Code Review Q#115042, answer score: 2
Revisions (0)
No revisions yet.