Skip to content

Commit 23e4f61

Browse files
cominterncomintern
committed
Fix code writing logic in EncapsulateFieldDialog. Corrected Get for Variants with both Let and Set. Closes #2500 
1 parent af3b2f3 commit 23e4f61

File tree

3 files changed

+81
-90
lines changed

3 files changed

+81
-90
lines changed

‎RetailCoder.VBE/Refactorings/EncapsulateField/EncapsulateFieldPresenter.cs

Lines changed: 27 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
using System.Linq;
22
using System.Windows.Forms;
33
using Antlr4.Runtime;
4+
using Rubberduck.Parsing;
45
using Rubberduck.Parsing.Grammar;
6+
using Rubberduck.Parsing.Symbols;
57

68
namespace Rubberduck.Refactorings.EncapsulateField
79
{
@@ -21,42 +23,20 @@ public EncapsulateFieldPresenter(IEncapsulateFieldDialog view, EncapsulateFieldM
2123
_model = model;
2224
}
2325

24-
private static readonly string[] PrimitiveTypes =
25-
{
26-
Tokens.Boolean,
27-
Tokens.Byte,
28-
Tokens.Date,
29-
Tokens.Decimal,
30-
Tokens.Double,
31-
Tokens.Long,
32-
Tokens.LongLong,
33-
Tokens.LongPtr,
34-
Tokens.Integer,
35-
Tokens.Single,
36-
Tokens.String,
37-
Tokens.StrPtr
38-
};
39-
4026
public EncapsulateFieldModel Show()
4127
{
4228
if (_model.TargetDeclaration == null) { return null; }
4329

4430
_view.TargetDeclaration = _model.TargetDeclaration;
4531
_view.NewPropertyName = _model.TargetDeclaration.IdentifierName;
4632

33+
var isVariant = _model.TargetDeclaration.AsTypeName.Equals(Tokens.Variant);
34+
var isValueType = !isVariant && (SymbolList.ValueTypes.Contains(_model.TargetDeclaration.AsTypeName) ||
35+
_model.TargetDeclaration.DeclarationType == DeclarationType.Enumeration);
36+
4737
if (_model.TargetDeclaration.References.Any(r => r.IsAssignment))
4838
{
49-
if (PrimitiveTypes.Contains(_model.TargetDeclaration.AsTypeName))
50-
{
51-
_view.MustImplementLetSetterType = true;
52-
_view.CanImplementSetSetterType = false;
53-
}
54-
else if (_model.TargetDeclaration.AsTypeName != Tokens.Variant)
55-
{
56-
_view.MustImplementSetSetterType = true;
57-
_view.CanImplementLetSetterType = false;
58-
}
59-
else
39+
if (isVariant)
6040
{
6141
RuleContext node = _model.TargetDeclaration.References.First(r => r.IsAssignment).Context;
6242
while (!(node is VBAParser.LetStmtContext) && !(node is VBAParser.SetStmtContext))
@@ -66,25 +46,36 @@ public EncapsulateFieldModel Show()
6646

6747
if (node is VBAParser.LetStmtContext)
6848
{
69-
_view.MustImplementLetSetterType = true;
70-
_view.CanImplementSetSetterType = false;
49+
_view.CanImplementLetSetterType = true;
7150
}
7251
else
7352
{
74-
_view.MustImplementSetSetterType = true;
75-
_view.CanImplementLetSetterType = false;
76-
}
53+
_view.CanImplementSetSetterType = true;
54+
}
55+
}
56+
else if (isValueType)
57+
{
58+
_view.CanImplementLetSetterType = true;
59+
}
60+
else
61+
{
62+
_view.CanImplementSetSetterType = true;
7763
}
7864
}
7965
else
8066
{
81-
if (PrimitiveTypes.Contains(_model.TargetDeclaration.AsTypeName))
67+
if (isValueType)
68+
{
69+
_view.CanImplementLetSetterType = true;
70+
}
71+
else if (!isVariant)
8272
{
83-
_view.CanImplementSetSetterType = false;
73+
_view.CanImplementSetSetterType = true;
8474
}
85-
else if (_model.TargetDeclaration.AsTypeName != Tokens.Variant)
75+
else
8676
{
87-
_view.CanImplementLetSetterType = false;
77+
_view.CanImplementLetSetterType = true;
78+
_view.CanImplementSetSetterType = true;
8879
}
8980
}
9081

‎RetailCoder.VBE/Refactorings/EncapsulateField/IEncapsulateFieldDialog.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@ public interface IEncapsulateFieldDialog :IDialogView
1111
bool CanImplementLetSetterType { get; set; }
1212
bool CanImplementSetSetterType { get; set; }
1313

14-
bool MustImplementLetSetterType { get; set; }
15-
bool MustImplementSetSetterType { get; set; }
14+
bool MustImplementLetSetterType { get; }
15+
bool MustImplementSetSetterType { get; }
1616

1717
string ParameterName { get; set; }
1818
}

‎RetailCoder.VBE/UI/Refactorings/EncapsulateFieldDialog.cs

Lines changed: 52 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -29,56 +29,18 @@ public string ParameterName
2929

3030
public Declaration TargetDeclaration { get; set; }
3131

32-
public bool CanImplementLetSetterType
33-
{
34-
get { return LetSetterTypeCheckBox.Enabled; }
35-
set
36-
{
37-
if (!value)
38-
{
39-
LetSetterTypeCheckBox.Checked = false;
40-
}
41-
LetSetterTypeCheckBox.Enabled = value;
42-
}
43-
}
32+
public bool CanImplementLetSetterType { get; set; }
4433

45-
public bool CanImplementSetSetterType
46-
{
47-
get { return SetSetterTypeCheckBox.Enabled; }
48-
set
49-
{
50-
if (!value)
51-
{
52-
SetSetterTypeCheckBox.Checked = false;
53-
}
54-
SetSetterTypeCheckBox.Enabled = value;
55-
}
56-
}
34+
public bool CanImplementSetSetterType { get; set; }
5735

5836
public bool MustImplementLetSetterType
5937
{
60-
get { return LetSetterTypeCheckBox.Checked; }
61-
set
62-
{
63-
if (value)
64-
{
65-
LetSetterTypeCheckBox.Checked = true;
66-
}
67-
LetSetterTypeCheckBox.Enabled = !value;
68-
}
38+
get { return CanImplementLetSetterType && !CanImplementSetSetterType; }
6939
}
7040

7141
public bool MustImplementSetSetterType
7242
{
73-
get { return SetSetterTypeCheckBox.Checked; }
74-
set
75-
{
76-
if (value)
77-
{
78-
SetSetterTypeCheckBox.Checked = true;
79-
}
80-
SetSetterTypeCheckBox.Enabled = !value;
81-
}
43+
get { return CanImplementSetSetterType && !CanImplementLetSetterType; }
8244
}
8345

8446
public EncapsulateFieldDialog(RubberduckParserState state, IIndenter indenter)
@@ -90,10 +52,7 @@ public EncapsulateFieldDialog(RubberduckParserState state, IIndenter indenter)
9052

9153
PropertyNameTextBox.TextChanged += PropertyNameBox_TextChanged;
9254
ParameterNameTextBox.TextChanged += VariableNameBox_TextChanged;
93-
94-
LetSetterTypeCheckBox.CheckedChanged += EncapsulateFieldDialog_SetterTypeChanged;
95-
SetSetterTypeCheckBox.CheckedChanged += EncapsulateFieldDialog_SetterTypeChanged;
96-
55+
9756
Shown += EncapsulateFieldDialog_Shown;
9857
}
9958

@@ -116,11 +75,36 @@ private void LocalizeDialog()
11675

11776
void EncapsulateFieldDialog_Shown(object sender, EventArgs e)
11877
{
78+
if (MustImplementSetSetterType)
79+
{
80+
SetSetterTypeCheckBox.Checked = true;
81+
DisableAssignmentSelection();
82+
}
83+
else
84+
{
85+
LetSetterTypeCheckBox.Checked = true;
86+
if (MustImplementLetSetterType)
87+
{
88+
DisableAssignmentSelection();
89+
}
90+
else
91+
{
92+
LetSetterTypeCheckBox.CheckedChanged += EncapsulateFieldDialog_SetterTypeChanged;
93+
SetSetterTypeCheckBox.CheckedChanged += EncapsulateFieldDialog_SetterTypeChanged;
94+
}
95+
}
96+
11997
ValidatePropertyName();
12098
ValidateVariableName();
12199
UpdatePreview();
122100
}
123101

102+
private void DisableAssignmentSelection()
103+
{
104+
LetSetterTypeCheckBox.Enabled = false;
105+
SetSetterTypeCheckBox.Enabled = false;
106+
}
107+
124108
private void PropertyNameBox_TextChanged(object sender, EventArgs e)
125109
{
126110
ValidatePropertyName();
@@ -142,10 +126,7 @@ private string GetPropertyText()
142126
{
143127
if (TargetDeclaration == null) { return string.Empty; }
144128

145-
var getterText = string.Join(Environment.NewLine,
146-
string.Format("Public Property Get {0}() As {1}", NewPropertyName, TargetDeclaration.AsTypeName),
147-
string.Format(" {0}{1} = {2}", MustImplementSetSetterType || !CanImplementLetSetterType ? "Set " : string.Empty, NewPropertyName, TargetDeclaration.IdentifierName),
148-
"End Property");
129+
var getterText = GenerateGetter();
149130

150131
var letterText = string.Join(Environment.NewLine,
151132
string.Format(Environment.NewLine + Environment.NewLine + "Public Property Let {0}(ByVal {1} As {2})",
@@ -160,13 +141,32 @@ private string GetPropertyText()
160141
"End Property");
161142

162143
var propertyText = getterText +
163-
(MustImplementLetSetterType ? letterText : string.Empty) +
164-
(MustImplementSetSetterType ? setterText : string.Empty);
144+
(LetSetterTypeCheckBox.Checked ? letterText : string.Empty) +
145+
(SetSetterTypeCheckBox.Checked ? setterText : string.Empty);
165146

166147
var propertyTextLines = propertyText.Split(new[] { Environment.NewLine }, StringSplitOptions.None);
167148
return string.Join(Environment.NewLine, _indenter.Indent(propertyTextLines));
168149
}
169150

151+
private string GenerateGetter()
152+
{
153+
if (SetSetterTypeCheckBox.Checked && LetSetterTypeCheckBox.Checked)
154+
{
155+
return string.Join(Environment.NewLine,
156+
string.Format("Public Property Get {0}() As {1}", NewPropertyName, TargetDeclaration.AsTypeName),
157+
string.Format(" If VarType({0}) = vbObject Then", TargetDeclaration.IdentifierName),
158+
string.Format(" Set {0} = {1}", NewPropertyName, TargetDeclaration.IdentifierName),
159+
" Else",
160+
string.Format(" {0} = {1}", NewPropertyName, TargetDeclaration.IdentifierName),
161+
" End If",
162+
"End Property");
163+
}
164+
return string.Join(Environment.NewLine,
165+
string.Format("Public Property Get {0}() As {1}", NewPropertyName, TargetDeclaration.AsTypeName),
166+
string.Format(" {0}{1} = {2}", SetSetterTypeCheckBox.Checked ? "Set " : string.Empty, NewPropertyName, TargetDeclaration.IdentifierName),
167+
"End Property");
168+
}
169+
170170
private void ValidatePropertyName()
171171
{
172172
InvalidPropertyNameIcon.Visible = ValidateName(NewPropertyName, ParameterName) ||

0 commit comments

Comments
 (0)