Skip to content

Commit b55e23a

Browse files
BZngrBZngr
committed
Improvements to ObjectStateUDT handling
Supports selection of an existing UDT to wrap fields. Provide some logic to establish re-use of a previously created objectStateUDT if the user adds more fields after the initial encapsulation process.
1 parent 36d51f6 commit b55e23a

13 files changed

+257
-69
lines changed

‎Rubberduck.Refactorings/EncapsulateField/EncapsulateFieldElementFactory.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,11 @@ public IEncapsulateFieldCandidate CreateEncapsulationCandidate(Declaration targe
5252

5353
udtVariable.AddMember(candidateUDTMember);
5454
}
55+
56+
var udtVariablesOfSameType = _declarationFinderProvider.DeclarationFinder.UserDeclarations(DeclarationType.Variable)
57+
.Where(v => v.AsTypeDeclaration == udtDeclaration);
58+
59+
udtVariable.CanBeObjectStateUDT = udtVariable.TypeDeclarationIsPrivate && udtVariablesOfSameType.Count() == 1;
5560
}
5661

5762
_validator.RegisterFieldCandidate(candidate);

‎Rubberduck.Refactorings/EncapsulateField/EncapsulateFieldModel.cs

Lines changed: 17 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
using System.Collections.Generic;
33
using System.Linq;
44
using Rubberduck.Parsing.Symbols;
5+
using Rubberduck.Refactorings.EncapsulateField.Extensions;
56
using Rubberduck.VBEditor;
67

78
namespace Rubberduck.Refactorings.EncapsulateField
@@ -23,7 +24,7 @@ public EncapsulateFieldModel(Declaration target, IEnumerable<IEncapsulateFieldCa
2324
_newObjectStateUDT = stateUDTField;
2425

2526
EncapsulationCandidates = candidates.ToList();
26-
//StateUDTField = stateUDTField;
27+
StateUDTField = stateUDTField;
2728
}
2829

2930
public List<IEncapsulateFieldCandidate> EncapsulationCandidates { set; get; } = new List<IEncapsulateFieldCandidate>();
@@ -49,33 +50,29 @@ public IEncapsulateFieldCandidate this[Declaration fieldDeclaration]
4950

5051
public bool EncapsulateWithUDT { set; get; }
5152

52-
private IObjectStateUDT _stateUDTField;
53-
public IObjectStateUDT StateUDTField
53+
public IObjectStateUDT StateUDTField { set; get; }
54+
55+
public string PreviewRefactoring() => _previewDelegate(this);
56+
57+
private List<IObjectStateUDT> _objStateCandidates;
58+
public IEnumerable<IObjectStateUDT> ObjectStateUDTCandidates
5459
{
55-
set
56-
{
57-
_stateUDTField = value;
58-
}
5960
get
6061
{
61-
if (!EncapsulateWithUDT) { return null; }
62-
63-
if (_stateUDTField != null)
62+
if (_objStateCandidates != null)
6463
{
65-
return _stateUDTField;
64+
return _objStateCandidates;
6665
}
6766

68-
var selectedStateUDT = EncapsulationCandidates.Where(sfc => sfc is IUserDefinedTypeCandidate udt
69-
&& udt.IsObjectStateUDT).Select(sfc => sfc as IUserDefinedTypeCandidate).FirstOrDefault();
70-
71-
_stateUDTField = selectedStateUDT != null
72-
? new ObjectStateUDT(selectedStateUDT)
73-
: _newObjectStateUDT;
67+
_objStateCandidates = new List<IObjectStateUDT>();
68+
foreach (var candidate in UDTFieldCandidates.Where(udt => udt.CanBeObjectStateUDT))
69+
{
70+
_objStateCandidates.Add(new ObjectStateUDT(candidate));
71+
}
7472

75-
return _stateUDTField;
73+
_objStateCandidates.Add(_newObjectStateUDT);
74+
return _objStateCandidates;
7675
}
7776
}
78-
79-
public string PreviewRefactoring() => _previewDelegate(this);
8077
}
8178
}

‎Rubberduck.Refactorings/EncapsulateField/EncapsulateFieldRefactoring.cs

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,10 +92,18 @@ protected override EncapsulateFieldModel InitializeModel(Declaration target)
9292
var selected = candidates.Single(c => c.Declaration == target);
9393
selected.EncapsulateFlag = true;
9494

95+
//var newObjectStateUDT = _encapsulationCandidateFactory.CreateStateUDTField();
96+
97+
if (!TryRetrieveExistingObjectStateUDT(target, candidates, out var objectStateUDT))
98+
{
99+
objectStateUDT = _encapsulationCandidateFactory.CreateStateUDTField();
100+
objectStateUDT.IsSelected = true;
101+
}
102+
95103
Model = new EncapsulateFieldModel(
96104
target,
97105
candidates,
98-
_encapsulationCandidateFactory.CreateStateUDTField(),
106+
objectStateUDT,
99107
PreviewRewrite,
100108
validator);
101109

@@ -107,6 +115,34 @@ protected override EncapsulateFieldModel InitializeModel(Declaration target)
107115
return Model;
108116
}
109117

118+
//Identify an existing objectStateUDT and make it unavailable for the user to select for encapsulation.
119+
//This prevents the user from inadvertently nesting a stateUDT within a new stateUDT
120+
private bool TryRetrieveExistingObjectStateUDT(Declaration target, IEnumerable<IEncapsulateFieldCandidate> candidates, out IObjectStateUDT objectStateUDT)
121+
{
122+
objectStateUDT = null;
123+
//Determination relies on matching the refactoring-generated name and a couple other UDT attributes
124+
//to determine if an objectStateUDT pre-exists the refactoring.
125+
126+
//Question: would using an Annotations (like '@IsObjectStateUDT) be better?
127+
//The logic would then be: if Annotated => it's the one. else => apply the matching criteria below
128+
129+
//e.g., In cases where the user chooses an existing UDT for the initial encapsulation, the matching
130+
//refactoring will not assign the name and the criteria below will fail => so applying an Annotation would
131+
//make it possible to find again
132+
var objectStateUDTIdentifier = $"{EncapsulateFieldResources.StateUserDefinedTypeIdentifierPrefix}{target.QualifiedModuleName.ComponentName}";
133+
134+
var objectStateUDTMatches = candidates.Where(c => c is IUserDefinedTypeCandidate udt
135+
&& udt.Declaration.HasPrivateAccessibility()
136+
&& udt.Declaration.AsTypeDeclaration.IdentifierName.StartsWith(objectStateUDTIdentifier, StringComparison.InvariantCultureIgnoreCase))
137+
.Select(pm => pm as IUserDefinedTypeCandidate);
138+
139+
if (objectStateUDTMatches.Count() == 1)
140+
{
141+
objectStateUDT = new ObjectStateUDT(objectStateUDTMatches.First()) { IsSelected = true };
142+
}
143+
return objectStateUDT != null;
144+
}
145+
110146
protected override void RefactorImpl(EncapsulateFieldModel model)
111147
{
112148
var refactorRewriteSession = new EncapsulateFieldRewriteSession(RewritingManager.CheckOutCodePaneSession()) as IEncapsulateFieldRewriteSession;

‎Rubberduck.Refactorings/EncapsulateField/FieldCandidates/ArrayCandidate.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ public override void LoadFieldReferenceContextReplacements()
3535
{
3636
var replacementText = RequiresAccessQualification(idRef)
3737
? $"{QualifiedModuleName.ComponentName}.{ReferenceForPreExistingReferences}"
38-
: FieldIdentifier;
38+
: ReferenceForPreExistingReferences;
3939

4040
SetReferenceRewriteContent(idRef, replacementText);
4141
}

‎Rubberduck.Refactorings/EncapsulateField/FieldCandidates/EncapsulateFieldCandidate.cs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -205,8 +205,8 @@ public virtual string PropertyName
205205
public override bool Equals(object obj)
206206
{
207207
return obj != null
208-
&& obj is IEncapsulateFieldCandidate
209-
&& obj.GetHashCode() == GetHashCode();
208+
&& obj is IEncapsulateFieldCandidate efc
209+
&& $"{efc.QualifiedModuleName.Name}.{efc.TargetID}" == $"{_qmn.Name}.{IdentifierName}";
210210
}
211211

212212
public override int GetHashCode() => _hashCode;
@@ -309,7 +309,8 @@ protected virtual IPropertyGeneratorAttributes AsPropertyAttributeSet
309309
ParameterName = ParameterName,
310310
GenerateLetter = ImplementLet,
311311
GenerateSetter = ImplementSet,
312-
UsesSetAssignment = Declaration.IsObject
312+
UsesSetAssignment = Declaration.IsObject,
313+
IsUDTProperty = false
313314
};
314315
}
315316
}

‎Rubberduck.Refactorings/EncapsulateField/FieldCandidates/ObjectStateUDT.cs

Lines changed: 41 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ public interface IObjectStateUDT : IEncapsulateFieldDeclaration
2020
void AddMembers(IEnumerable<IEncapsulateFieldCandidate> fields);
2121
bool IsExistingDeclaration { get; }
2222
Declaration AsTypeDeclaration { get; }
23+
bool IsSelected { set; get; }
24+
bool IsEncapsulateFieldCandidate(IEncapsulateFieldCandidate efc);
2325
}
2426

2527
//ObjectStateUDT can be an existing UDT (Private only) selected by the user, or a
@@ -29,17 +31,20 @@ public class ObjectStateUDT : IObjectStateUDT
2931
private static string _defaultNewFieldName = EncapsulateFieldResources.DefaultStateUDTFieldName;
3032
private List<IEncapsulateFieldCandidate> _members;
3133
private readonly IUserDefinedTypeCandidate _wrappedUDT;
34+
private int _hashCode;
3235

3336
public ObjectStateUDT(IUserDefinedTypeCandidate udt)
34-
:this(udt.Declaration.AsTypeName)
37+
: this(udt.Declaration.AsTypeName)
3538
{
36-
if (!udt.Declaration.Accessibility.Equals(Accessibility.Private))
39+
if (!udt.TypeDeclarationIsPrivate)
3740
{
3841
throw new ArgumentException();
3942
}
4043

44+
FieldIdentifier = udt.IdentifierName;
4145
_wrappedUDT = udt;
42-
QualifiedModuleName = udt.Declaration.QualifiedModuleName;
46+
udt.EncapsulateFlag = false;
47+
_hashCode = ($"{_qmn.Name}.{_wrappedUDT.IdentifierName}").GetHashCode();
4348
}
4449

4550
public ObjectStateUDT(QualifiedModuleName qmn)
@@ -59,13 +64,20 @@ private ObjectStateUDT(string typeIdentifier)
5964

6065
public string AsTypeName => _wrappedUDT?.AsTypeName ?? TypeIdentifier;
6166

62-
public QualifiedModuleName QualifiedModuleName { set; get; }
67+
public bool IsSelected { set; get; }
68+
69+
private QualifiedModuleName _qmn;
70+
public QualifiedModuleName QualifiedModuleName
71+
{
72+
set => _qmn = value;
73+
get => _wrappedUDT?.QualifiedModuleName ?? _qmn;
74+
}
6375

6476
public string TypeIdentifier { set; get; }
6577

6678
public bool IsExistingDeclaration => _wrappedUDT != null;
6779

68-
public Declaration AsTypeDeclaration => _wrappedUDT.Declaration.AsTypeDeclaration;
80+
public Declaration AsTypeDeclaration => _wrappedUDT?.Declaration.AsTypeDeclaration;
6981

7082
public string FieldIdentifier { set; get; }
7183

@@ -90,6 +102,30 @@ public string TypeDeclarationBlock(IIndenter indenter = null)
90102
return string.Join(Environment.NewLine, BlockLines(Accessibility.Private));
91103
}
92104

105+
public override bool Equals(object obj)
106+
{
107+
if (obj is IObjectStateUDT stateUDT && stateUDT.FieldIdentifier == FieldIdentifier)
108+
{
109+
return true;
110+
}
111+
if (obj is IEncapsulateFieldDeclaration fd && fd.IdentifierName == IdentifierName)
112+
{
113+
return true;
114+
}
115+
return false;
116+
}
117+
118+
public bool IsEncapsulateFieldCandidate(IEncapsulateFieldCandidate efc)
119+
{
120+
if (efc is IEncapsulateFieldDeclaration fd && fd.IdentifierName == IdentifierName)
121+
{
122+
return true;
123+
}
124+
return false;
125+
}
126+
127+
public override int GetHashCode() => _hashCode;
128+
93129
private IEnumerable<string> BlockLines(Accessibility accessibility)
94130
{
95131
var blockLines = new List<string>();

‎Rubberduck.Refactorings/EncapsulateField/FieldCandidates/UserDefinedTypeCandidate.cs

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ public interface IUserDefinedTypeCandidate : IEncapsulateFieldCandidate
1414
IEnumerable<IUserDefinedTypeMemberCandidate> Members { get; }
1515
void AddMember(IUserDefinedTypeMemberCandidate member);
1616
bool TypeDeclarationIsPrivate { set; get; }
17-
bool IsObjectStateUDT { set; get; }
17+
bool CanBeObjectStateUDT { set; get; }
1818
}
1919

2020
public class UserDefinedTypeCandidate : EncapsulateFieldCandidate, IUserDefinedTypeCandidate
@@ -36,11 +36,11 @@ public void AddMember(IUserDefinedTypeMemberCandidate member)
3636

3737
public bool TypeDeclarationIsPrivate { set; get; }
3838

39-
private bool _isObjectStateUDT;
40-
public bool IsObjectStateUDT
39+
private bool _canBeObjectStateUDT;
40+
public bool CanBeObjectStateUDT
4141
{
42-
set => _isObjectStateUDT = value;
43-
get => _isObjectStateUDT;
42+
set => _canBeObjectStateUDT = value;
43+
get => _canBeObjectStateUDT;
4444
}
4545

4646
public override string FieldIdentifier
@@ -101,7 +101,7 @@ public override bool EncapsulateFlag
101101
}
102102
base.EncapsulateFlag = value;
103103
}
104-
get => _encapsulateFlag && !_isObjectStateUDT;
104+
get => _encapsulateFlag; // && !_isObjectStateUDT;
105105
}
106106

107107
public override void LoadFieldReferenceContextReplacements()
@@ -203,6 +203,24 @@ public override bool TryValidateEncapsulationAttributes(out string errorMessage)
203203
return true;
204204
}
205205

206+
protected override IPropertyGeneratorAttributes AsPropertyAttributeSet
207+
{
208+
get
209+
{
210+
return new PropertyAttributeSet()
211+
{
212+
PropertyName = PropertyName,
213+
BackingField = ReferenceWithinNewProperty,
214+
AsTypeName = AsTypeName_Property,
215+
ParameterName = ParameterName,
216+
GenerateLetter = ImplementLet,
217+
GenerateSetter = ImplementSet,
218+
UsesSetAssignment = Declaration.IsObject,
219+
IsUDTProperty = true
220+
};
221+
}
222+
}
223+
206224
private void LoadPrivateUDTFieldLocalReferenceExpressions()
207225
{
208226
foreach (var idRef in Declaration.References)

‎Rubberduck.Refactorings/EncapsulateField/PropertyGenerator.cs

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
using Rubberduck.SmartIndenter;
1+
using Rubberduck.Parsing.Grammar;
2+
using Rubberduck.SmartIndenter;
23
using System;
34
using System.Collections.Generic;
45

@@ -13,6 +14,7 @@ public interface IPropertyGeneratorAttributes
1314
bool GenerateLetter { get; }
1415
bool GenerateSetter { get; }
1516
bool UsesSetAssignment { get; }
17+
bool IsUDTProperty { get; }
1618
}
1719

1820
public class PropertyAttributeSet : IPropertyGeneratorAttributes
@@ -24,6 +26,7 @@ public class PropertyAttributeSet : IPropertyGeneratorAttributes
2426
public bool GenerateLetter { get; set; }
2527
public bool GenerateSetter { get; set; }
2628
public bool UsesSetAssignment { get; set; }
29+
public bool IsUDTProperty { get; set; }
2730
}
2831

2932
public class PropertyGenerator
@@ -37,6 +40,7 @@ public PropertyGenerator() { }
3740
public bool GenerateLetter { get; set; }
3841
public bool GenerateSetter { get; set; }
3942
public bool UsesSetAssignment { get; set; }
43+
public bool IsUDTProperty { get; set; }
4044

4145
public string AsPropertyBlock(IPropertyGeneratorAttributes spec, IIndenter indenter)
4246
{
@@ -47,6 +51,7 @@ public string AsPropertyBlock(IPropertyGeneratorAttributes spec, IIndenter inden
4751
GenerateLetter = spec.GenerateLetter;
4852
GenerateSetter = spec.GenerateSetter;
4953
UsesSetAssignment = spec.UsesSetAssignment;
54+
IsUDTProperty = spec.IsUDTProperty;
5055
return string.Join(Environment.NewLine, indenter.Indent(AsLines, true));
5156
}
5257

@@ -104,8 +109,11 @@ private string LetterCode
104109
{
105110
return string.Empty;
106111
}
112+
113+
var byVal_byRef = IsUDTProperty ? Tokens.ByRef : Tokens.ByVal;
114+
107115
return string.Join(Environment.NewLine,
108-
$"Public Property Let {PropertyName}(ByVal {ParameterName} As {AsTypeName})",
116+
$"Public Property Let {PropertyName}({byVal_byRef} {ParameterName} As {AsTypeName})",
109117
$" {BackingField} = {ParameterName}",
110118
"End Property",
111119
Environment.NewLine);

‎RubberduckTests/Refactoring/EncapsulateField/EncapsulateFieldTests.cs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -744,7 +744,11 @@ End Property
744744
Assert.AreEqual(expectedCode.Trim(), actualCode);
745745
}
746746

747+
//Consider...
748+
//https://docs.microsoft.com/en-us/office/vba/language/reference/user-interface-help/can-t-redim-erase-or-assign-to-variant-that-contains-array-whose-element-is-with
749+
//https://docs.microsoft.com/en-us/office/vba/language/reference/user-interface-help/constants-fixed-length-strings-arrays-user-defined-types-and-declare-statements
747750
[Test]
751+
[Ignore("Resolve use of Redim locally and externally")]
748752
[Category("Refactorings")]
749753
[Category("Encapsulate Field")]
750754
public void EncapsulateArray_Redim()

‎RubberduckTests/Refactoring/EncapsulateField/EncapsulateFieldValidatorTests.cs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,7 @@ End Property
161161
Assert.IsFalse(model["fizz"].TryValidateEncapsulationAttributes(out _));
162162
}
163163

164+
164165
[TestCase("Number", "Bazzle", true, true)]
165166
[TestCase("Number", "Number", false, false)]
166167
[TestCase("Test", "Number", false, true)]
@@ -438,7 +439,7 @@ End Type
438439
var userInput = new UserInputDataObject()
439440
.UserSelectsField(fieldUT);
440441

441-
userInput.EncapsulateAsUDT = true;
442+
userInput.EncapsulateUsingUDTField();
442443

443444
var presenterAction = Support.SetParameters(userInput);
444445

@@ -481,7 +482,7 @@ Public mF|oo As Long
481482
var userInput = new UserInputDataObject()
482483
.UserSelectsField(fieldUT);
483484

484-
userInput.EncapsulateAsUDT = true;
485+
userInput.EncapsulateUsingUDTField();
485486

486487
var presenterAction = Support.SetParameters(userInput);
487488

0 commit comments

Comments
 (0)