Skip to content

Commit f4b0855

Browse files
grleachmangrleachman
committed
many refactorings, and hopefully a final fix to issue 844
1 parent b28d32e commit f4b0855

20 files changed

+1294
-778
lines changed

‎RetailCoder.VBE/Refactorings/ExtractMethod/ExtractMethodExtraction.cs

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,7 @@ public void apply(ICodeModuleWrapper codeModule, IExtractMethodModel model, Sele
1616
var newMethodCall = model.Method.NewMethodCall();
1717
var positionToInsertNewMethod = model.PositionForNewMethod;
1818
var positionForMethodCall = model.PositionForMethodCall;
19-
var selectionToRemove = model.SelectionToRemove;
20-
19+
var selectionToRemove = model.RowsToRemove;
2120
// The next 4 lines are dependent on the positions of the various parts,
2221
// so have to be applied in the correct order.
2322
var newMethod = constructLinesOfProc(codeModule, model);
@@ -28,46 +27,40 @@ public void apply(ICodeModuleWrapper codeModule, IExtractMethodModel model, Sele
2827

2928
public virtual void removeSelection(ICodeModuleWrapper codeModule, IEnumerable<Selection> selection)
3029
{
31-
foreach (var item in selection)
30+
foreach (var item in selection.OrderBy(x => -x.StartLine))
3231
{
3332
var start = item.StartLine;
3433
var end = item.EndLine;
3534
var lineCount = end - start + 1;
36-
3735
codeModule.DeleteLines(start,lineCount);
38-
3936
}
4037
}
38+
4139
public virtual string constructLinesOfProc(ICodeModuleWrapper codeModule, IExtractMethodModel model)
4240
{
4341

4442
var newLine = Environment.NewLine;
4543
var method = model.Method;
4644
var keyword = Tokens.Sub;
4745
var asTypeClause = string.Empty;
48-
var selection = model.SelectionToRemove;
49-
46+
var selection = model.RowsToRemove;
5047

5148
var access = method.Accessibility.ToString();
5249
var extractedParams = method.Parameters.Select(p => ExtractedParameter.PassedBy.ByRef + " " + p.Name + " " + Tokens.As + " " + p.TypeName);
5350
var parameters = "(" + string.Join(", ", extractedParams) + ")";
54-
5551
//method signature
5652
var result = access + ' ' + keyword + ' ' + method.MethodName + parameters + ' ' + asTypeClause + newLine;
57-
5853
// method body
5954
string textToMove = "";
6055
foreach (var item in selection)
6156
{
6257
textToMove += codeModule.get_Lines(item.StartLine, item.EndLine - item.StartLine + 1);
6358
textToMove += Environment.NewLine;
6459
}
65-
6660
// method end;
6761
result += textToMove;
6862
result += Tokens.End + " " + Tokens.Sub;
6963
return result;
7064
}
71-
7265
}
7366
}

‎RetailCoder.VBE/Refactorings/ExtractMethod/ExtractMethodModel.cs

Lines changed: 99 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -12,111 +12,128 @@
1212
using Rubberduck.VBEditor;
1313
using Rubberduck.VBEditor.Extensions;
1414

15-
namespace Rubberduck.Refactorings.ExtractMethod
15+
16+
public static class IEnumerableExt
1617
{
18+
/// <summary>
19+
/// Yields an Enumeration of selector Type,
20+
/// by checking for gaps between elements
21+
/// using the supplied increment function to work out the next value
22+
/// </summary>
23+
/// <typeparam name="T"></typeparam>
24+
/// <typeparam name="U"></typeparam>
25+
/// <param name="inputs"></param>
26+
/// <param name="getIncr"></param>
27+
/// <param name="selector"></param>
28+
/// <param name="comparisonFunc"></param>
29+
/// <returns></returns>
30+
public static IEnumerable<U> GroupByMissing<T, U>(this IEnumerable<T> inputs, Func<T, T> getIncr, Func<T, T, U> selector, Func<T, T, int> comparisonFunc)
31+
{
32+
33+
var initialized = false;
34+
T first = default(T);
35+
T last = default(T);
36+
T next = default(T);
37+
Tuple<T, T> tuple = null;
1738

39+
foreach (var input in inputs)
40+
{
41+
if (!initialized)
42+
{
43+
first = input;
44+
last = input;
45+
initialized = true;
46+
continue;
47+
}
48+
if (comparisonFunc(last, input) < 0)
49+
{
50+
throw new ArgumentException(string.Format("Values are not monotonically increasing. {0} should be less than {1}", last, input));
51+
}
52+
var inc = getIncr(last);
53+
if (!input.Equals(inc))
54+
{
55+
yield return selector(first, last);
56+
first = input;
57+
}
58+
last = input;
59+
}
60+
if (initialized)
61+
{
62+
yield return selector(first, last);
63+
}
64+
}
65+
}
66+
67+
namespace Rubberduck.Refactorings.ExtractMethod
68+
{
1869
public class ExtractMethodModel : IExtractMethodModel
1970
{
20-
private const string NEW_METHOD = "NewMethod";
71+
private List<Declaration> _extractDeclarations;
72+
private IExtractMethodParameterClassification _paramClassify;
73+
private IExtractedMethod _extractedMethod;
2174

22-
public ExtractMethodModel(List<IExtractMethodRule> emRules, IExtractedMethod extractedMethod)
75+
public ExtractMethodModel(IExtractedMethod extractedMethod, IExtractMethodParameterClassification paramClassify)
2376
{
24-
_rules = emRules;
2577
_extractedMethod = extractedMethod;
78+
_paramClassify = paramClassify;
2679
}
2780

28-
2981
public void extract(IEnumerable<Declaration> declarations, QualifiedSelection selection, string selectedCode)
3082
{
3183
var items = declarations.ToList();
32-
var sourceMember = items.FindSelectedDeclaration(selection, DeclarationExtensions.ProcedureTypes, d => ((ParserRuleContext)d.Context.Parent).GetSelection());
84+
_selection = selection;
85+
_selectedCode = selectedCode;
86+
_rowsToRemove = new List<Selection>();
87+
88+
var sourceMember = items.FindSelectedDeclaration(
89+
selection,
90+
DeclarationExtensions.ProcedureTypes,
91+
d => ((ParserRuleContext)d.Context.Parent).GetSelection());
92+
3393
if (sourceMember == null)
3494
{
3595
throw new InvalidOperationException("Invalid selection.");
3696
}
3797

3898
var inScopeDeclarations = items.Where(item => item.ParentScope == sourceMember.Scope).ToList();
39-
40-
_byref = new List<Declaration>();
41-
_byval = new List<Declaration>();
42-
_declarationsToMove = new List<Declaration>();
43-
_extractDeclarations = new List<Tuple<Declaration, bool>>();
44-
_extractedMethod = new ExtractedMethod();
45-
46-
47-
var selectionToRemove = new List<Selection>();
4899
var selectionStartLine = selection.Selection.StartLine;
49100
var selectionEndLine = selection.Selection.EndLine;
50-
51101
var methodInsertLine = sourceMember.Context.Stop.Line + 1;
102+
52103
_positionForNewMethod = new Selection(methodInsertLine, 1, methodInsertLine, 1);
53104

54-
// https://github.com/rubberduck-vba/Rubberduck/wiki/Extract-Method-Refactoring-%3A-Workings---Determining-what-params-to-move
55105
foreach (var item in inScopeDeclarations)
56106
{
57-
var flags = new Byte();
58-
59-
foreach (var oRef in item.References)
60-
{
61-
foreach (var rule in _rules)
62-
{
63-
rule.setValidFlag(ref flags, oRef, selection.Selection);
64-
}
65-
}
66-
67-
//TODO: extract this to seperate class.
68-
if (flags < 4) { /*ignore the variable*/ }
69-
else if (flags < 12)
70-
_byref.Add(item);
71-
else if (flags == 12)
72-
_declarationsToMove.Add(item);
73-
else if (flags > 12)
74-
_byval.Add(item);
75-
76-
if (flags >= 18)
77-
{
78-
_extractDeclarations.Add(Tuple.Create(item, true));
79-
}
80-
107+
_paramClassify.classifyDeclarations(selection, item);
81108
}
109+
_declarationsToMove = _paramClassify.DeclarationsToMove.ToList();
82110

111+
_rowsToRemove = splitSelection(selection.Selection, _declarationsToMove).ToList();
83112

84-
_declarationsToMove.ForEach(d => selectionToRemove.Add(d.Selection));
85-
selectionToRemove.Add(selection.Selection);
86-
87-
var methodCallPositionStartLine = selectionStartLine - selectionToRemove.Count(s => s.StartLine < selectionStartLine);
113+
var methodCallPositionStartLine = selectionStartLine - _declarationsToMove.Count(d => d.Selection.StartLine < selectionStartLine);
88114
_positionForMethodCall = new Selection(methodCallPositionStartLine, 1, methodCallPositionStartLine, 1);
89-
90-
var methodParams = _byref.Select(dec => new ExtractedParameter(dec.AsTypeName, ExtractedParameter.PassedBy.ByRef, dec.IdentifierName))
91-
.Union(_byval.Select(dec => new ExtractedParameter(dec.AsTypeName, ExtractedParameter.PassedBy.ByVal, dec.IdentifierName)));
92-
93-
// iterate until we have a non-clashing method name.
94-
var newMethodName = NEW_METHOD;
95-
96-
var newMethodInc = 0;
97-
while (declarations.FirstOrDefault(d =>
98-
DeclarationExtensions.ProcedureTypes.Contains(d.DeclarationType)
99-
&& d.IdentifierName.Equals(newMethodName)) != null)
100-
{
101-
newMethodInc++;
102-
newMethodName = NEW_METHOD + newMethodInc;
103-
}
104-
105-
_extractedMethod.MethodName = newMethodName;
106115
_extractedMethod.ReturnValue = null;
107116
_extractedMethod.Accessibility = Accessibility.Private;
108117
_extractedMethod.SetReturnValue = false;
109-
_extractedMethod.Parameters = methodParams.ToList();
110-
111-
_selection = selection;
112-
_selectedCode = selectedCode;
113-
_selectionToRemove = selectionToRemove.ToList();
118+
_extractedMethod.Parameters = _paramClassify.ExtractedParameters.ToList();
114119

115120
}
116121

117-
private List<Declaration> _byref;
118-
private List<Declaration> _byval;
119-
private List<Declaration> _moveIn;
122+
public IEnumerable<Selection> splitSelection(Selection selection, IEnumerable<Declaration> declarations)
123+
{
124+
var tupleList = new List<Tuple<int, int>>();
125+
var declarationRows = declarations
126+
.Where(decl =>
127+
selection.StartLine <= decl.Selection.StartLine &&
128+
decl.Selection.StartLine <= selection.EndLine)
129+
.Select(decl => decl.Selection.StartLine)
130+
.OrderBy(x => x)
131+
.ToList();
132+
133+
var gappedSelectionRows = Enumerable.Range(selection.StartLine, selection.EndLine - selection.StartLine + 1).Except(declarationRows).ToList();
134+
var returnList = gappedSelectionRows.GroupByMissing(x => (x + 1), (x, y) => new Selection(x, 1, y, 1), (x, y) => y - x);
135+
return returnList;
136+
}
120137

121138
private Declaration _sourceMember;
122139
public Declaration SourceMember { get { return _sourceMember; } }
@@ -132,34 +149,33 @@ public void extract(IEnumerable<Declaration> declarations, QualifiedSelection se
132149

133150
private IEnumerable<ExtractedParameter> _input;
134151
public IEnumerable<ExtractedParameter> Inputs { get { return _input; } }
135-
136152
private IEnumerable<ExtractedParameter> _output;
137153
public IEnumerable<ExtractedParameter> Outputs { get { return _output; } }
138154

139155
private List<Declaration> _declarationsToMove;
140156
public IEnumerable<Declaration> DeclarationsToMove { get { return _declarationsToMove; } }
141157

142-
private IExtractedMethod _extractedMethod;
143-
144-
private IEnumerable<IExtractMethodRule> _rules;
145-
146158
public IExtractedMethod Method { get { return _extractedMethod; } }
147159

148-
149160
private Selection _positionForMethodCall;
150161
public Selection PositionForMethodCall { get { return _positionForMethodCall; } }
151162

152163
public string NewMethodCall { get { return _extractedMethod.NewMethodCall(); } }
153164

154165
private Selection _positionForNewMethod;
155166
public Selection PositionForNewMethod { get { return _positionForNewMethod; } }
156-
IEnumerable<Selection> _selectionToRemove;
157-
private List<IExtractMethodRule> emRules;
158-
159-
public IEnumerable<Selection> SelectionToRemove { get { return _selectionToRemove; } }
160-
161-
private List<Tuple<Declaration, bool>> _extractDeclarations;
162-
public IEnumerable<Tuple<Declaration, bool>> ExtractDeclarations { get { return _extractDeclarations; } }
167+
IList<Selection> _rowsToRemove;
168+
public IEnumerable<Selection> RowsToRemove
169+
{
170+
// we need to split selectionToRemove around any declarations that
171+
// are within the selection.
172+
get { return _declarationsToMove.Select(decl => decl.Selection).Union(_rowsToRemove)
173+
.Select( x => new Selection(x.StartLine,1,x.EndLine,1)) ; }
174+
}
163175

176+
public IEnumerable<Declaration> DeclarationsToExtract
177+
{
178+
get { return _extractDeclarations; }
179+
}
164180
}
165181
}

‎RetailCoder.VBE/Refactorings/ExtractMethod/ExtractMethodParameterClassification.cs

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
using System;
2+
using System.Collections.Generic;
3+
using System.Linq;
4+
using System.Text;
5+
using System.Threading.Tasks;
6+
using Rubberduck.Parsing.Symbols;
7+
using Rubberduck.VBEditor;
8+
9+
namespace Rubberduck.Refactorings.ExtractMethod
10+
{
11+
public class ExtractMethodParameterClassification : IExtractMethodParameterClassification
12+
{
13+
// https://github.com/rubberduck-vba/Rubberduck/wiki/Extract-Method-Refactoring-%3A-Workings---Determining-what-params-to-move
14+
private readonly IEnumerable<IExtractMethodRule> _emRules;
15+
private List<Declaration> _byref;
16+
private List<Declaration> _byval;
17+
private List<Declaration> _declarationsToMove;
18+
private List<Declaration> _extractDeclarations;
19+
20+
public ExtractMethodParameterClassification(IEnumerable<IExtractMethodRule> emRules)
21+
{
22+
_emRules = emRules;
23+
_byref = new List<Declaration>();
24+
_byval = new List<Declaration>();
25+
_declarationsToMove = new List<Declaration>();
26+
_extractDeclarations = new List<Declaration>();
27+
}
28+
29+
public void classifyDeclarations(QualifiedSelection selection, Declaration item)
30+
{
31+
32+
byte flags = new Byte();
33+
foreach (var oRef in item.References)
34+
{
35+
foreach (var rule in _emRules)
36+
{
37+
var byteFlag = rule.setValidFlag(oRef, selection.Selection);
38+
flags = (byte)(flags | (byte)byteFlag);
39+
40+
}
41+
}
42+
43+
if (flags < 4) { /*ignore the variable*/ }
44+
else if (flags < 12)
45+
_byref.Add(item);
46+
else if (flags == 12)
47+
_declarationsToMove.Add(item);
48+
else if (flags > 12)
49+
_byval.Add(item);
50+
51+
if (flags >= 18)
52+
{
53+
_extractDeclarations.Add(item);
54+
}
55+
}
56+
57+
public IEnumerable<ExtractedParameter> ExtractedParameters
58+
{
59+
get {
60+
return _byref.Select(dec => new ExtractedParameter(dec.AsTypeName, ExtractedParameter.PassedBy.ByRef, dec.IdentifierName)).
61+
Union(_byval.Select(dec => new ExtractedParameter(dec.AsTypeName, ExtractedParameter.PassedBy.ByVal, dec.IdentifierName)));
62+
}
63+
}
64+
65+
public IEnumerable<Declaration> DeclarationsToMove { get { return _declarationsToMove; } }
66+
public IEnumerable<Declaration> ExtractedDeclarations { get { return _extractDeclarations; } }
67+
68+
}
69+
}

0 commit comments

Comments
 (0)