Skip to content

Commit

Permalink
Address #5556 Review comments
Browse files Browse the repository at this point in the history
Renaming per suggestions.  Also, clean-up of commented code and removal
of non-required 'using' directives.
  • Loading branch information
BZngr committed Aug 14, 2020
1 parent 1b9073d commit 70edc91
Show file tree
Hide file tree
Showing 11 changed files with 126 additions and 114 deletions.
6 changes: 3 additions & 3 deletions Rubberduck.Refactorings/Common/CodeBuilder.cs
Expand Up @@ -72,12 +72,12 @@ public interface ICodeBuilder
/// Generates a default RHS property parameter IdentifierName
/// </summary>
/// <param name="propertyIdentifier">Let/Set Property IdentifierName</param>
string DefaultPropertyValueParamIdentifier(string propertyIdentifier);
string BuildPropertyRhsParameterName(string propertyIdentifier);
}

public class CodeBuilder : ICodeBuilder
{
public string DefaultPropertyValueParamIdentifier(string propertyIdentifier)
public string BuildPropertyRhsParameterName(string propertyIdentifier)
=> string.Format(Resources.Refactorings.Refactorings.CodeBuilder_DefaultPropertyRHSParamFormat, propertyIdentifier.ToLowerCaseFirstLetter());

public string BuildMemberBlockFromPrototype(ModuleBodyElementDeclaration declaration,
Expand Down Expand Up @@ -119,7 +119,7 @@ public bool TryBuildPropertySetCodeBlock(Declaration prototype, string propertyI
return false;
}

var propertyValueParam = parameterIdentifier ?? DefaultPropertyValueParamIdentifier(propertyIdentifier);
var propertyValueParam = parameterIdentifier ?? BuildPropertyRhsParameterName(propertyIdentifier);

var asType = prototype.IsArray
? $"{Tokens.Variant}"
Expand Down
Expand Up @@ -111,7 +111,7 @@ private IEncapsulateFieldCandidate CreateCandidate(Declaration target, IValidate
if (target.IsUserDefinedType())
{
var udtValidator = EncapsulateFieldValidationsProvider.NameOnlyValidator(NameValidators.UserDefinedType);
var udtField = new UserDefinedTypeCandidate(target, udtValidator, _codeBuilder.DefaultPropertyValueParamIdentifier) as IUserDefinedTypeCandidate;
var udtField = new UserDefinedTypeCandidate(target, udtValidator, _codeBuilder.BuildPropertyRhsParameterName) as IUserDefinedTypeCandidate;

(Declaration udtDeclaration, IEnumerable<Declaration> udtMembers) = GetUDTAndMembersForField(udtField);

Expand All @@ -126,7 +126,7 @@ private IEncapsulateFieldCandidate CreateCandidate(Declaration target, IValidate
{
udtMemberValidator = EncapsulateFieldValidationsProvider.NameOnlyValidator(NameValidators.UserDefinedTypeMemberArray);
}
var candidateUDTMember = new UserDefinedTypeMemberCandidate(CreateCandidate(udtMemberDeclaration, udtMemberValidator), udtField, _codeBuilder.DefaultPropertyValueParamIdentifier) as IUserDefinedTypeMemberCandidate;
var candidateUDTMember = new UserDefinedTypeMemberCandidate(CreateCandidate(udtMemberDeclaration, udtMemberValidator), udtField, _codeBuilder.BuildPropertyRhsParameterName) as IUserDefinedTypeMemberCandidate;

udtField.AddMember(candidateUDTMember);
}
Expand All @@ -142,10 +142,10 @@ private IEncapsulateFieldCandidate CreateCandidate(Declaration target, IValidate
}
else if (target.IsArray)
{
return new ArrayCandidate(target, validator, _codeBuilder.DefaultPropertyValueParamIdentifier);
return new ArrayCandidate(target, validator, _codeBuilder.BuildPropertyRhsParameterName);
}

var candidate = new EncapsulateFieldCandidate(target, validator, _codeBuilder.DefaultPropertyValueParamIdentifier);
var candidate = new EncapsulateFieldCandidate(target, validator, _codeBuilder.BuildPropertyRhsParameterName);
return candidate;
}

Expand Down
@@ -1,5 +1,4 @@
using Antlr4.Runtime;
using Rubberduck.Parsing;
using Rubberduck.Parsing;
using Rubberduck.Parsing.Grammar;
using Rubberduck.Parsing.Symbols;
using Rubberduck.Resources;
Expand All @@ -17,8 +16,8 @@ public interface IArrayCandidate : IEncapsulateFieldCandidate
public class ArrayCandidate : EncapsulateFieldCandidate, IArrayCandidate
{
private string _subscripts;
public ArrayCandidate(Declaration declaration, IValidateVBAIdentifiers validator, Func<string,string> paramNameBuilder)
:base(declaration, validator, paramNameBuilder)
public ArrayCandidate(Declaration declaration, IValidateVBAIdentifiers validator, Func<string,string> parameterNameBuilder)
:base(declaration, validator, parameterNameBuilder)
{
ImplementLet = false;
ImplementSet = false;
Expand Down
Expand Up @@ -45,13 +45,13 @@ public class EncapsulateFieldCandidate : IEncapsulateFieldCandidate
protected int _hashCode;
private string _identifierName;
protected EncapsulationIdentifiers _fieldAndProperty;
private Func<string, string> _paramNameBuilder;
private Func<string, string> _parameterNameBuilder;

public EncapsulateFieldCandidate(Declaration declaration, IValidateVBAIdentifiers identifierValidator, Func<string, string> paramNameBuilder)
public EncapsulateFieldCandidate(Declaration declaration, IValidateVBAIdentifiers identifierValidator, Func<string, string> parameterNameBuilder)
{
_target = declaration;
NameValidator = identifierValidator;
_paramNameBuilder = paramNameBuilder;
_parameterNameBuilder = parameterNameBuilder;

_fieldAndProperty = new EncapsulationIdentifiers(declaration.IdentifierName, identifierValidator);
IdentifierName = declaration.IdentifierName;
Expand Down Expand Up @@ -202,7 +202,7 @@ public string IdentifierName

public virtual string ReferenceQualifier { set; get; }

public string ParameterName => _paramNameBuilder(PropertyIdentifier);
public string ParameterName => _parameterNameBuilder(PropertyIdentifier);

private bool _implLet;
public bool ImplementLet { get => !IsReadOnly && _implLet; set => _implLet = value; }
Expand Down
@@ -1,12 +1,8 @@
using Antlr4.Runtime;
using Rubberduck.Common;
using Rubberduck.Parsing;
using Rubberduck.Parsing.Grammar;
using Rubberduck.Parsing.Grammar;
using Rubberduck.Parsing.Symbols;
using Rubberduck.Refactorings.Common;
using System;
using System.Collections.Generic;
using System.Linq;

namespace Rubberduck.Refactorings.EncapsulateField
{
Expand All @@ -21,8 +17,8 @@ public interface IUserDefinedTypeCandidate : IEncapsulateFieldCandidate

public class UserDefinedTypeCandidate : EncapsulateFieldCandidate, IUserDefinedTypeCandidate
{
public UserDefinedTypeCandidate(Declaration declaration, IValidateVBAIdentifiers identifierValidator, Func<string,string> paramNameBuilder)
: base(declaration, identifierValidator, paramNameBuilder)
public UserDefinedTypeCandidate(Declaration declaration, IValidateVBAIdentifiers identifierValidator, Func<string,string> parameterNameBuilder)
: base(declaration, identifierValidator, parameterNameBuilder)
{
}

Expand Down
Expand Up @@ -20,11 +20,11 @@ public class UserDefinedTypeMemberCandidate : IUserDefinedTypeMemberCandidate
{
private int _hashCode;
private readonly string _uniqueID;
private Func<string, string> _paramNameBuilder;
public UserDefinedTypeMemberCandidate(IEncapsulateFieldCandidate candidate, IUserDefinedTypeCandidate udtField, Func<string,string> paramNameBuilder)
private Func<string, string> _parameterNameBuilder;
public UserDefinedTypeMemberCandidate(IEncapsulateFieldCandidate candidate, IUserDefinedTypeCandidate udtField, Func<string,string> parameterNameBuilder)
{
_wrappedCandidate = candidate;
_paramNameBuilder = paramNameBuilder;
_parameterNameBuilder = parameterNameBuilder;
UDTField = udtField;
PropertyIdentifier = IdentifierName;
BackingIdentifier = IdentifierName;
Expand Down Expand Up @@ -203,7 +203,7 @@ public QualifiedModuleName QualifiedModuleName

public string PropertyAsTypeName => _wrappedCandidate.PropertyAsTypeName;

public string ParameterName => _paramNameBuilder(PropertyIdentifier);
public string ParameterName => _parameterNameBuilder(PropertyIdentifier);

public bool ImplementLet => _wrappedCandidate.ImplementLet;

Expand Down
2 changes: 1 addition & 1 deletion RubberduckTests/CodeBuilderTests.cs
Expand Up @@ -339,7 +339,7 @@ private string ParseAndTest<T>(string inputCode, string targetIdentifier, Declar

private static string PropertyGetBlockFromPrototypeTest<T>(T target, PropertyBlockFromPrototypeParams testParams) where T : Declaration
{
new CodeBuilder().TryBuildPropertyGetCodeBlock(target, testParams.Identifier, out string result, testParams.Accessibility, testParams.Content); //, testParams.WriteParam);
new CodeBuilder().TryBuildPropertyGetCodeBlock(target, testParams.Identifier, out string result, testParams.Accessibility, testParams.Content);
return result;
}

Expand Down
Expand Up @@ -19,7 +19,7 @@ namespace RubberduckTests.Refactoring.EncapsulateField
{
public class EncapsulateFieldTestSupport : InteractiveRefactoringTestBase<IEncapsulateFieldPresenter, EncapsulateFieldModel>
{
public static string ParamNameBuilder(string property) => $"{property.ToLowerCaseFirstLetter()}Value";
public string RhsParameterNameBuilder(string property) => $"{property.ToLowerCaseFirstLetter()}Value";

public string StateUDTDefaultType => $"T{MockVbeBuilder.TestModuleName}";

Expand Down
Expand Up @@ -21,8 +21,6 @@ public class EncapsulateFieldTests : InteractiveRefactoringTestBase<IEncapsulate
{
private EncapsulateFieldTestSupport Support { get; } = new EncapsulateFieldTestSupport();

private static Func<string, string> Param = EncapsulateFieldTestSupport.ParamNameBuilder;

[TestCase("fizz", true, "baz", true, "buzz", true)]
[TestCase("fizz", false, "baz", true, "buzz", true)]
[TestCase("fizz", false, "baz", false, "buzz", true)]
Expand Down Expand Up @@ -69,10 +67,12 @@ public class EncapsulateFieldTests : InteractiveRefactoringTestBase<IEncapsulate

foreach (var variable in encapsulated)
{
var rhsParameterName = Support.RhsParameterNameBuilder($"{variable}Prop");

StringAssert.Contains($"Private {variable} As", actualCode);
StringAssert.Contains($"{variable}Prop = {variable}", actualCode);
StringAssert.Contains($"{variable} = {Param($"{variable}Prop")}", actualCode);
StringAssert.Contains($"Let {variable}Prop(ByVal {Param($"{variable}Prop")} As", actualCode);
StringAssert.Contains($"{variable} = {rhsParameterName}", actualCode);
StringAssert.Contains($"Let {variable}Prop(ByVal {rhsParameterName} As", actualCode);
StringAssert.Contains($"Property Get {variable}Prop()", actualCode);
}
}
Expand Down Expand Up @@ -133,10 +133,12 @@ public class EncapsulateFieldTests : InteractiveRefactoringTestBase<IEncapsulate
{
if (flags[key])
{
var rhsParameterName = Support.RhsParameterNameBuilder($"{key}Prop");

StringAssert.Contains($"Private {key} As", actualCode);
StringAssert.Contains($"{key}Prop = {key}", actualCode);
StringAssert.Contains($"{key} = {Param($"{key}Prop")}", actualCode);
StringAssert.Contains($"Let {key}Prop(ByVal {Param($"{key}Prop")} As", actualCode);
StringAssert.Contains($"{key} = {rhsParameterName}", actualCode);
StringAssert.Contains($"Let {key}Prop(ByVal {rhsParameterName} As", actualCode);
StringAssert.Contains($"Property Get {key}Prop()", actualCode);
}
}
Expand Down Expand Up @@ -232,10 +234,11 @@ End Property
End Property";

var presenterAction = Support.SetParametersForSingleTarget("fizz", "Name");
var rhsParameterName = Support.RhsParameterNameBuilder("Name");

var actualCode = Support.RefactoredCode(inputCode.ToCodeString(), presenterAction);
Assert.Greater(actualCode.IndexOf($"fizz = {Param("Name")}"), actualCode.IndexOf("fizz As Integer"));
Assert.Less(actualCode.IndexOf($"fizz = {Param("Name")}"), actualCode.IndexOf("Get Foo"));
Assert.Greater(actualCode.IndexOf($"fizz = {rhsParameterName}"), actualCode.IndexOf("fizz As Integer"));
Assert.Less(actualCode.IndexOf($"fizz = {rhsParameterName}"), actualCode.IndexOf("Get Foo"));
}

[TestCase("|Public fizz As Integer\r\nPublic buzz As Boolean", "Private fizz As Integer\r\nPublic buzz As Boolean")]
Expand Down Expand Up @@ -294,12 +297,14 @@ public void EncapsulatePrivateField()
@"|Private fizz As Integer";

var presenterAction = Support.SetParametersForSingleTarget("fizz", "Name");
var rhsParameterName = Support.RhsParameterNameBuilder("Name");

var actualCode = Support.RefactoredCode(inputCode.ToCodeString(), presenterAction);
StringAssert.Contains("Public Property Get Name() As Integer", actualCode);
StringAssert.Contains("Public Property Let Name(", actualCode);
StringAssert.Contains($"(ByVal {Param("Name")} As Integer)", actualCode);
StringAssert.Contains($"(ByVal {rhsParameterName} As Integer)", actualCode);
StringAssert.Contains("Name = fizz", actualCode);
StringAssert.Contains($"fizz = {Param("Name")}", actualCode);
StringAssert.Contains($"fizz = {rhsParameterName}", actualCode);
StringAssert.Contains("End Property", actualCode);
}

Expand Down Expand Up @@ -365,28 +370,17 @@ public void EncapsulatePrivateFieldInList()
const string inputCode =
@"Private fi|zz As Integer, fuzz As Integer, fazz As Integer";

// const string expectedCode =
// @"
//Private fizz_1 As Integer, fuzz As Integer, fazz As Integer

//Public Property Get Fizz() As Integer
// Fizz = fizz_1
//End Property

//Public Property Let Fizz(ByVal value As Integer)
// fizz_1 = value
//End Property
//";
var presenterAction = Support.SetParametersForSingleTarget("fizz");
var rhsParameterName = Support.RhsParameterNameBuilder("Fizz");

var actualCode = Support.RefactoredCode(inputCode.ToCodeString(), presenterAction);
StringAssert.Contains("Private fizz_1 As Integer, fuzz As Integer,", actualCode);
StringAssert.Contains("Public Property Get Fizz() As Integer", actualCode);
StringAssert.Contains("Public Property Let Fizz(", actualCode);
StringAssert.Contains($"(ByVal {Param("Fizz")} As Integer)", actualCode);
StringAssert.Contains($"(ByVal {rhsParameterName} As Integer)", actualCode);
StringAssert.Contains("Fizz = fizz_1", actualCode);
StringAssert.Contains($"fizz_1 = {Param("Fizz")}", actualCode);
StringAssert.Contains($"fizz_1 = {rhsParameterName}", actualCode);
StringAssert.Contains("End Property", actualCode);
//Assert.AreEqual(expectedCode.Trim(), actualCode);
}

[Test]
Expand All @@ -397,26 +391,16 @@ public void EncapsulatePrivateField_Defaults()
const string inputCode =
@"|Private fizz As Integer";

// const string expectedCode =
// @"Private fizz_1 As Integer

//Public Property Get Fizz() As Integer
// Fizz = fizz_1
//End Property

//Public Property Let Fizz(ByVal value As Integer)
// fizz_1 = value
//End Property
//";
var presenterAction = Support.UserAcceptsDefaults();
var rhsParameterName = Support.RhsParameterNameBuilder("Fizz");

var actualCode = Support.RefactoredCode(inputCode.ToCodeString(), presenterAction);
StringAssert.Contains("Public Property Get Fizz() As Integer", actualCode);
StringAssert.Contains("Public Property Let Fizz(", actualCode);
StringAssert.Contains($"(ByVal {Param("Fizz")} As Integer)", actualCode);
StringAssert.Contains($"(ByVal {rhsParameterName} As Integer)", actualCode);
StringAssert.Contains("Fizz = fizz_1", actualCode);
StringAssert.Contains($"fizz_1 = {Param("Fizz")}", actualCode);
StringAssert.Contains($"fizz_1 = {rhsParameterName}", actualCode);
StringAssert.Contains("End Property", actualCode);
//Assert.AreEqual(expectedCode.Trim(), actualCode);
}

[Test]
Expand All @@ -428,10 +412,12 @@ public void EncapsulatePrivateField_DefaultsAsUDT()
@"|Private fizz As Integer";

var presenterAction = Support.UserAcceptsDefaults(convertFieldToUDTMember: true);
var rhsParameterName = Support.RhsParameterNameBuilder("Fizz");

var actualCode = Support.RefactoredCode(inputCode.ToCodeString(), presenterAction);
StringAssert.Contains("Fizz As Integer", actualCode);
StringAssert.Contains($"this As {Support.StateUDTDefaultType}", actualCode);
StringAssert.Contains($"this.Fizz = {Param("Fizz")}", actualCode);
StringAssert.Contains($"this.Fizz = {rhsParameterName}", actualCode);
}

[Test]
Expand All @@ -454,14 +440,15 @@ Sub Bar(ByVal name As Integer)

var validator = EncapsulateFieldValidationsProvider.NameOnlyValidator(NameValidators.Default);
var enapsulationIdentifiers = new EncapsulationIdentifiers("fizz", validator) { Property = "Name" };
var rhsParameterName = Support.RhsParameterNameBuilder("Name");

var actualCode = Support.RefactoredCode(inputCode.ToCodeString(), presenterAction);
StringAssert.AreEqualIgnoringCase(enapsulationIdentifiers.TargetFieldName, "fizz");
StringAssert.Contains($"Private {enapsulationIdentifiers.TargetFieldName} As Integer", actualCode);
StringAssert.Contains("Property Get Name", actualCode);
StringAssert.Contains("Property Let Name", actualCode);
StringAssert.Contains($"Name = {enapsulationIdentifiers.TargetFieldName}", actualCode);
StringAssert.Contains($"{enapsulationIdentifiers.TargetFieldName} = {Param("Name")}", actualCode);
StringAssert.Contains($"{enapsulationIdentifiers.TargetFieldName} = {rhsParameterName}", actualCode);
}

[Test]
Expand Down

0 comments on commit 70edc91

Please sign in to comment.