Consolidate Signature and Member content generation code #5419
Conversation
Adds method and property generation code to extension classes. Allows removal two files/classes and duplicate function/content in EncapsulateField refactoring and ImplementInterface refactoring. Moves Encapsulate Field Declaration Extensions to RefactoringCommon folder to share content with ImplementInterface.
Codecov Report
@@ Coverage Diff @@
## next #5419 +/- ##
==========================================
- Coverage 61.52% 61.49% -0.02%
==========================================
Files 1195 1194 -1
Lines 40572 40595 +23
Branches 8643 8666 +23
==========================================
+ Hits 24958 24963 +5
- Misses 13788 13789 +1
- Partials 1826 1843 +17
|
BZngr
added
the
PR-Status: Conflicting
|
Merging this PR should wait for the same reason(s) as given for #5420 |
BZngr
added
Status: Blocked by other issue
and removed
PR-Status: Conflicting
|
It's been 14 days, I'm not going to hold up merging this PR - I'll just deal with any conflicts on my side =) |
| /// <param name="content"></param> | ||
| /// <param name="parameterIdentifier"></param> | ||
| /// <returns></returns> | ||
| public static string FieldToPropertyBlock(this Declaration variable, DeclarationType letSetGetType, string propertyIdentifier, string accessibility = null, string content = null, string parameterIdentifier = null) |
There was a problem hiding this comment.
I'm not sure this type of work really belongs here as an extension method to Declaration... would the target object not always be a VariableDeclaration?
There was a problem hiding this comment.
Yes - should have been VariableDeclaration. Once the dust settles surrounding EncapsulateField - I'll put together a PR for this.
There was a problem hiding this comment.
I think this does not belong into an extension at all. Instead, I thing there should be some ICodeBulder to build the standard code for declarations. That would make the standard easily testable and would also allow to specify different versions of the code by injecting another implementation.
The same applies to the extensions for module body elements, even more, actually.
There was a problem hiding this comment.
@MDoerner I was certain using an extension was a debatable location for this functionality - and I was hoping for either confirmation or some guidance to move it elsewhere - thanks. As I understand your comment, the ICodeBuilder instance would be delivered by CW?
As per my earlier comment, I was planning to wait for the coming EncapsulateField PR before introducing anything further changes.
| @@ -1779,4 +1779,7 @@ Import aborted.</value> | |||
| <data name="EncapsulateField_WrapFieldsInPrivateType" xml:space="preserve"> | |||
| <value>Wrap Fields in Private Type</value> | |||
| </data> | |||
| <data name="ImplementInterface_TODO" xml:space="preserve"> | |||
| <value>'TODO implement interface member</value> | |||
There was a problem hiding this comment.
This does not belong here; it is not related to the UI.
Refactorings that generate code (e.g.,
ImplementInterface, EncapsulateField, and others) have a common interest in creating blocks of code for Properties and Methods based on, or using, existing Member and Variable declarations. This common interest has resulted in similar (if not identical) code showing up in multiple refactorings and in UI elements as well (e.g., just to display the signature). So, as I was pondering how to avoid duplicating the code in a fourth location (MoveMemberRefactoring), it seemed that it was time to attempt a PR that would consolidate this functionality in a central location.In its current state, the PR introduces this functionality through
DeclarationExtensionandModuleBodyElementDeclarationExtensionmethods in theRubberduck.Refactorings.Commonfolder. This approach/location can certainly be changed. The primary goal was to corral this capability into a single place where it could be easily reused by the refactorings. And, I figured it would be easier for all to evaluate where this code belongs if there was a working version from which to start.Notes:
The PR primarily affects the
EncapsulateFieldRefactoringandAddInterfaceImplementationsRefactoringActionrefactorings.There is a UI converter class
DeclarationToMemberSignatureConverterwhich also duplicates the code to build a full member signature from theDeclaration. It seems that the UI project does not have access to theRubberduck.Refactorings.Commonnamespace(?) - so for now, it is left as-is.Update: I am unable to find any references to
DeclarationToMemberSignatureConverterin the xaml files (or anywhere else). I think the file/class can be deleted.The functions in
DeclarationExtensions.csfor this PR begin withFieldToPropertyBlocknear the end of the file. There are other functions starting at the top of the file that were moved there fromEncapulateFieldRefactoring. A few are relevant to support this PR. I've moved the others there in preparation for theMoveMemberRefactoring.