Skip to content

Commit 51ea7df

Browse files
bclothierbclothier
committed
Clean up of EncapsulateField objects, make the presenter dumber
Encapsulate the logic that don't depend on UI in model rather than the view model Closes #4677 by fixing the insertion logic for new field, a new unit test
1 parent bf17684 commit 51ea7df

File tree

5 files changed

+168
-154
lines changed

5 files changed

+168
-154
lines changed

‎Rubberduck.Core/UI/Refactorings/EncapsulateField/EncapsulateFieldPresenter.cs

Lines changed: 6 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -1,96 +1,22 @@
1-
using System.Linq;
2-
using Antlr4.Runtime;
3-
using Rubberduck.Parsing;
4-
using Rubberduck.Parsing.Grammar;
5-
using Rubberduck.Parsing.Symbols;
6-
using Rubberduck.Refactorings;
1+
using Rubberduck.Refactorings;
72
using Rubberduck.Refactorings.EncapsulateField;
83

94
namespace Rubberduck.UI.Refactorings.EncapsulateField
105
{
11-
internal class EncapsulateFieldPresenter : RefactoringPresenterBase<EncapsulateFieldModel, EncapsulateFieldDialog, EncapsulateFieldView, EncapsulateFieldViewModel>, IEncapsulateFieldPresenter
6+
internal class EncapsulateFieldPresenter : RefactoringPresenterBase<EncapsulateFieldModel, IRefactoringDialog<EncapsulateFieldModel, IRefactoringView<EncapsulateFieldModel>, IRefactoringViewModel<EncapsulateFieldModel>>, IRefactoringView<EncapsulateFieldModel>, IRefactoringViewModel<EncapsulateFieldModel>>, IEncapsulateFieldPresenter
127
{
138
public EncapsulateFieldPresenter(EncapsulateFieldModel model,
14-
IRefactoringDialogFactory dialogFactory) : base(model, dialogFactory)
15-
{
16-
ViewModel = dialogFactory.CreateViewModel<EncapsulateFieldModel, EncapsulateFieldViewModel>(model);
17-
}
18-
19-
public override EncapsulateFieldViewModel ViewModel { get; }
9+
IRefactoringDialogFactory dialogFactory) : base(model, dialogFactory) { }
2010

2111
public override EncapsulateFieldModel Show()
2212
{
23-
if (Model.TargetDeclaration == null) { return null; }
24-
25-
ViewModel.TargetDeclaration = Model.TargetDeclaration;
26-
27-
var isVariant = Model.TargetDeclaration.AsTypeName.Equals(Tokens.Variant);
28-
var isValueType = !isVariant && (SymbolList.ValueTypes.Contains(Model.TargetDeclaration.AsTypeName) ||
29-
Model.TargetDeclaration.DeclarationType == DeclarationType.Enumeration);
30-
31-
AssignSetterAndLetterAvailability(isVariant, isValueType);
32-
33-
Dialog.ShowDialog();
34-
if (DialogResult != RefactoringDialogResult.Execute)
13+
if (Model.TargetDeclaration == null)
3514
{
3615
return null;
3716
}
3817

39-
Model.PropertyName = ViewModel.PropertyName;
40-
Model.ImplementLetSetterType = ViewModel.IsLetSelected;
41-
Model.ImplementSetSetterType = ViewModel.IsSetSelected;
42-
Model.CanImplementLet = ViewModel.CanHaveLet;
43-
44-
Model.ParameterName = ViewModel.ParameterName;
45-
return Model;
46-
}
47-
48-
private void AssignSetterAndLetterAvailability(bool isVariant, bool isValueType)
49-
{
50-
if (Model.TargetDeclaration.References.Any(r => r.IsAssignment))
51-
{
52-
if (isVariant)
53-
{
54-
RuleContext node = Model.TargetDeclaration.References.First(r => r.IsAssignment).Context;
55-
while (!(node is VBAParser.LetStmtContext) && !(node is VBAParser.SetStmtContext))
56-
{
57-
node = node.Parent;
58-
}
59-
60-
if (node is VBAParser.LetStmtContext)
61-
{
62-
ViewModel.CanHaveLet = true;
63-
}
64-
else
65-
{
66-
ViewModel.CanHaveSet = true;
67-
}
68-
}
69-
else if (isValueType)
70-
{
71-
ViewModel.CanHaveLet = true;
72-
}
73-
else
74-
{
75-
ViewModel.CanHaveSet = true;
76-
}
77-
}
78-
else
79-
{
80-
if (isValueType)
81-
{
82-
ViewModel.CanHaveLet = true;
83-
}
84-
else if (!isVariant)
85-
{
86-
ViewModel.CanHaveSet = true;
87-
}
88-
else
89-
{
90-
ViewModel.CanHaveLet = true;
91-
ViewModel.CanHaveSet = true;
92-
}
93-
}
18+
var model = base.Show();
19+
return DialogResult != RefactoringDialogResult.Execute ? null : model;
9420
}
9521
}
9622
}

‎Rubberduck.Core/UI/Refactorings/EncapsulateField/EncapsulateFieldViewModel.cs

Lines changed: 13 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -19,16 +19,15 @@ public EncapsulateFieldViewModel(EncapsulateFieldModel model, RubberduckParserSt
1919
Indenter = indenter;
2020

2121
IsLetSelected = true;
22-
CanHaveLet = true;
22+
PropertyName = model.TargetDeclaration.IdentifierName;
2323
}
2424

25-
private Declaration _targetDeclaration;
2625
public Declaration TargetDeclaration
2726
{
28-
get => _targetDeclaration;
27+
get => Model.TargetDeclaration;
2928
set
3029
{
31-
_targetDeclaration = value;
30+
Model.TargetDeclaration = value;
3231
PropertyName = value.IdentifierName;
3332
}
3433
}
@@ -45,73 +44,50 @@ public bool ExpansionState
4544
}
4645
}
4746

48-
private bool _canHaveLet;
49-
public bool CanHaveLet
50-
{
51-
get => _canHaveLet;
52-
set
53-
{
54-
_canHaveLet = value;
55-
OnPropertyChanged();
56-
}
57-
}
58-
59-
private bool _canHaveSet;
60-
public bool CanHaveSet
61-
{
62-
get => _canHaveSet;
63-
set
64-
{
65-
_canHaveSet = value;
66-
OnPropertyChanged();
67-
}
68-
}
47+
public bool CanHaveLet => Model.CanImplementLet;
48+
public bool CanHaveSet => Model.CanImplementSet;
6949

70-
private bool _isLetSelected;
7150
public bool IsLetSelected
7251
{
73-
get => _isLetSelected;
52+
get => Model.ImplementLetSetterType;
7453
set
7554
{
76-
_isLetSelected = value;
55+
Model.ImplementLetSetterType = value;
7756
OnPropertyChanged();
7857
OnPropertyChanged(nameof(PropertyPreview));
7958
}
8059
}
8160

82-
private bool _isSetSelected;
8361
public bool IsSetSelected
8462
{
85-
get => _isSetSelected;
63+
get => Model.ImplementSetSetterType;
8664
set
8765
{
88-
_isSetSelected = value;
66+
Model.ImplementSetSetterType = value;
8967
OnPropertyChanged();
9068
OnPropertyChanged(nameof(PropertyPreview));
9169
}
9270
}
9371

94-
private string _propertyName;
9572
public string PropertyName
9673
{
97-
get => _propertyName;
74+
get => Model.PropertyName;
9875
set
9976
{
100-
_propertyName = value;
77+
Model.PropertyName = value;
10178
OnPropertyChanged();
10279
OnPropertyChanged(nameof(IsValidPropertyName));
10380
OnPropertyChanged(nameof(HasValidNames));
10481
OnPropertyChanged(nameof(PropertyPreview));
10582
}
10683
}
10784

108-
private string _parameterName = "value";
10985
public string ParameterName
11086
{
111-
get => _parameterName;
87+
get => Model.ParameterName;
11288
set
11389
{
114-
_parameterName = value;
90+
Model.ParameterName = value;
11591
OnPropertyChanged();
11692
OnPropertyChanged(nameof(IsValidParameterName));
11793
OnPropertyChanged(nameof(HasValidNames));

‎Rubberduck.Refactorings/EncapsulateField/EncapsulateFieldModel.cs

Lines changed: 71 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
using System.Collections.Generic;
22
using System.Linq;
3+
using Antlr4.Runtime;
34
using Rubberduck.Common;
5+
using Rubberduck.Parsing;
6+
using Rubberduck.Parsing.Grammar;
47
using Rubberduck.Parsing.Symbols;
58
using Rubberduck.Parsing.VBA;
69
using Rubberduck.VBEditor;
@@ -11,20 +14,82 @@ public class EncapsulateFieldModel
1114
{
1215
public RubberduckParserState State { get; }
1316

14-
public Declaration TargetDeclaration { get; }
17+
public EncapsulateFieldModel(RubberduckParserState state, QualifiedSelection selection)
18+
{
19+
State = state;
20+
IList<Declaration> declarations = state.DeclarationFinder.UserDeclarations(DeclarationType.Variable).ToList();
21+
22+
TargetDeclaration = declarations.FindVariable(selection);
23+
ParameterName = "value";
24+
}
25+
26+
private Declaration _targetDeclaration;
27+
public Declaration TargetDeclaration
28+
{
29+
get => _targetDeclaration;
30+
set {
31+
_targetDeclaration = value;
32+
AssignSetterAndLetterAvailability();
33+
}
34+
}
1535

1636
public string PropertyName { get; set; }
1737
public string ParameterName { get; set; }
1838
public bool ImplementLetSetterType { get; set; }
1939
public bool ImplementSetSetterType { get; set; }
20-
public bool CanImplementLet { get; set; }
40+
public bool CanImplementLet { get; private set; }
41+
public bool CanImplementSet { get; private set; }
2142

22-
public EncapsulateFieldModel(RubberduckParserState state, QualifiedSelection selection)
43+
private void AssignSetterAndLetterAvailability()
2344
{
24-
State = state;
25-
IList<Declaration> declarations = state.DeclarationFinder.UserDeclarations(DeclarationType.Variable).ToList();
45+
var isVariant = _targetDeclaration.AsTypeName.Equals(Tokens.Variant);
46+
var isValueType = !isVariant && (SymbolList.ValueTypes.Contains(_targetDeclaration.AsTypeName) ||
47+
_targetDeclaration.DeclarationType == DeclarationType.Enumeration);
2648

27-
TargetDeclaration = declarations.FindVariable(selection);
49+
if (_targetDeclaration.References.Any(r => r.IsAssignment))
50+
{
51+
if (isVariant)
52+
{
53+
RuleContext node = _targetDeclaration.References.First(r => r.IsAssignment).Context;
54+
while (!(node is VBAParser.LetStmtContext) && !(node is VBAParser.SetStmtContext))
55+
{
56+
node = node.Parent;
57+
}
58+
59+
if (node is VBAParser.LetStmtContext)
60+
{
61+
CanImplementLet = true;
62+
}
63+
else
64+
{
65+
CanImplementSet = true;
66+
}
67+
}
68+
else if (isValueType)
69+
{
70+
CanImplementLet = true;
71+
}
72+
else
73+
{
74+
CanImplementSet = true;
75+
}
76+
}
77+
else
78+
{
79+
if (isValueType)
80+
{
81+
CanImplementLet = true;
82+
}
83+
else if (!isVariant)
84+
{
85+
CanImplementSet = true;
86+
}
87+
else
88+
{
89+
CanImplementLet = true;
90+
CanImplementSet = true;
91+
}
92+
}
2893
}
2994
}
3095
}

‎Rubberduck.Refactorings/EncapsulateField/EncapsulateFieldRefactoring.cs

Lines changed: 11 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,16 @@
22
using System;
33
using System.Collections.Generic;
44
using System.Linq;
5+
using Rubberduck.Parsing;
6+
using Rubberduck.Parsing.Grammar;
57
using Rubberduck.Parsing.Rewriter;
68
using Rubberduck.Parsing.Symbols;
79
using Rubberduck.Parsing.VBA;
810
using Rubberduck.VBEditor;
911
using Rubberduck.SmartIndenter;
1012
using Rubberduck.VBEditor.SafeComWrappers.Abstract;
1113
using Rubberduck.VBEditor.Utility;
14+
using Environment = System.Environment;
1215

1316
namespace Rubberduck.Refactorings.EncapsulateField
1417
{
@@ -105,20 +108,15 @@ private void AddProperty(IRewriteSession rewriteSession)
105108
var rewriter = rewriteSession.CheckOutModuleRewriter(_model.TargetDeclaration.QualifiedModuleName);
106109

107110
UpdateReferences(rewriteSession);
108-
SetFieldToPrivate(rewriter);
109-
111+
110112
var members = _model.State.DeclarationFinder
111113
.Members(_model.TargetDeclaration.QualifiedName.QualifiedModuleName)
112114
.OrderBy(declaration => declaration.QualifiedSelection);
113115

114116
var fields = members.Where(d => d.DeclarationType == DeclarationType.Variable && !d.ParentScopeDeclaration.DeclarationType.HasFlag(DeclarationType.Member)).ToList();
115117

116118
var property = Environment.NewLine + Environment.NewLine + GetPropertyText();
117-
if (members.Any(m => m.DeclarationType.HasFlag(DeclarationType.Member)))
118-
{
119-
property += Environment.NewLine;
120-
}
121-
119+
122120
if (_model.TargetDeclaration.Accessibility != Accessibility.Private)
123121
{
124122
var newField = $"Private {_model.TargetDeclaration.IdentifierName} As {_model.TargetDeclaration.AsTypeName}";
@@ -132,11 +130,15 @@ private void AddProperty(IRewriteSession rewriteSession)
132130

133131
if (_model.TargetDeclaration.Accessibility == Accessibility.Private || fields.Count > 1)
134132
{
133+
if (_model.TargetDeclaration.Accessibility != Accessibility.Private)
134+
{
135+
rewriter.Remove(_model.TargetDeclaration);
136+
}
135137
rewriter.InsertAfter(fields.Last().Context.Stop.TokenIndex, property);
136138
}
137139
else
138140
{
139-
rewriter.InsertBefore(0, property);
141+
rewriter.Replace(_model.TargetDeclaration.Context.GetAncestor<VBAParser.ModuleDeclarationsElementContext>(), property);
140142
}
141143
}
142144

@@ -148,15 +150,7 @@ private void UpdateReferences(IRewriteSession rewriteSession)
148150
rewriter.Replace(reference.Context, _model.PropertyName);
149151
}
150152
}
151-
152-
private void SetFieldToPrivate(IModuleRewriter rewriter)
153-
{
154-
if (_model.TargetDeclaration.Accessibility != Accessibility.Private)
155-
{
156-
rewriter.Remove(_model.TargetDeclaration);
157-
}
158-
}
159-
153+
160154
private string GetPropertyText()
161155
{
162156
var generator = new PropertyGenerator

0 commit comments

Comments
 (0)