New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extract Class #892

Open
retailcoder opened this Issue Dec 22, 2015 · 12 comments

Comments

Projects
None yet
3 participants
@retailcoder
Member

retailcoder commented Dec 22, 2015

We need a refactoring that helps VBA programmers to refactor procedural code into object-oriented code. Extracting methods is step one - extracting closely related methods into a class is step two.
#178 / Extract Interface would be step 3.

This refactoring works off the active code pane - it doesn't matter that we're in a standard, class, or form module. It displays a dialog that lists all members; user selects which members to move into the new class, enters a name for that new class, and Rubberduck...

  • Adds a new class module, names it as specified by the user
  • Adds a new private, self-initialized (As New Foobar) field in the original module, of the type of the extracted class.
  • Adds the selected members as public members of the extracted class
  • Replaces method calls with member calls (on the new field instance) in the original module

Hmm, perhaps the dialog also needs a place to specify a name for the private field.

@retailcoder retailcoder added this to the Version 2.0 milestone Dec 22, 2015

@Hosch250 Hosch250 self-assigned this Dec 31, 2015

@Hosch250

This comment has been minimized.

Show comment
Hide comment
@Hosch250

Hosch250 Dec 31, 2015

Member

How would the calls change? Would they all just change from Foo(param1, param2) to newField.Foo(param1, param2)?

Member

Hosch250 commented Dec 31, 2015

How would the calls change? Would they all just change from Foo(param1, param2) to newField.Foo(param1, param2)?

@Hosch250

This comment has been minimized.

Show comment
Hide comment
@Hosch250

Hosch250 Dec 31, 2015

Member

Also, what members are applicable? Fields, properties, methods? What about events? What about interface implementation members?

Member

Hosch250 commented Dec 31, 2015

Also, what members are applicable? Fields, properties, methods? What about events? What about interface implementation members?

@retailcoder

This comment has been minimized.

Show comment
Hide comment
@retailcoder

retailcoder Dec 31, 2015

Member

Anything in the module body: functions, procedures and properties.

Member

retailcoder commented Dec 31, 2015

Anything in the module body: functions, procedures and properties.

@retailcoder

This comment has been minimized.

Show comment
Hide comment
@retailcoder

retailcoder Dec 31, 2015

Member

Exclude interface members and event handlers... keep it simple :-)

Member

retailcoder commented Dec 31, 2015

Exclude interface members and event handlers... keep it simple :-)

@retailcoder

This comment has been minimized.

Show comment
Hide comment
@retailcoder

retailcoder Dec 31, 2015

Member

Call sites will need a new field if there's more than 1 call, or a new local if there's only 1 (make the object as tightly-scoped as possible) - you'll need to prompt for a name and replace calls with member calls on the new object.

Member

retailcoder commented Dec 31, 2015

Call sites will need a new field if there's more than 1 call, or a new local if there's only 1 (make the object as tightly-scoped as possible) - you'll need to prompt for a name and replace calls with member calls on the new object.

@Hosch250

This comment has been minimized.

Show comment
Hide comment
@Hosch250

Hosch250 Jan 1, 2016

Member

Could you give me a few compilable examples of the different syntaxes I need to use? Also, I am supposed to delete the existing items, correct? (If so, the rewriter will be perfect - remove them from the entire module, then rewrite the entire module in one blow.)

Are these examples correct?

' In module
Sub foo()
End Sub

Sub goo()
    foo
End Sub

Becomes:

' In module
Sub foo()
End Sub

Sub goo()
    Dim d As NewClass
    d.foo
End Sub

And

' Outside module
Sub goo()
    Dim d As OldClass
    d.foo
End Sub

Becomes:

' Outside module
Sub goo()
    Dim d As OldClass
    d.NewField.foo
End Sub

What about calls to the old class from the new class? Do I resolve and correct these, or just move the members verbatim?

Member

Hosch250 commented Jan 1, 2016

Could you give me a few compilable examples of the different syntaxes I need to use? Also, I am supposed to delete the existing items, correct? (If so, the rewriter will be perfect - remove them from the entire module, then rewrite the entire module in one blow.)

Are these examples correct?

' In module
Sub foo()
End Sub

Sub goo()
    foo
End Sub

Becomes:

' In module
Sub foo()
End Sub

Sub goo()
    Dim d As NewClass
    d.foo
End Sub

And

' Outside module
Sub goo()
    Dim d As OldClass
    d.foo
End Sub

Becomes:

' Outside module
Sub goo()
    Dim d As OldClass
    d.NewField.foo
End Sub

What about calls to the old class from the new class? Do I resolve and correct these, or just move the members verbatim?

@retailcoder

This comment has been minimized.

Show comment
Hide comment
@retailcoder

retailcoder Jan 1, 2016

Member

Yes, the extracted members should be removed from their original location, and call sites rewritten so that the code still compiles and runs just like before the refactoring.

If an extracted method has a dependency on a member that isn't selected, that member should be automatically selected - you don't want to introduce circular dependencies and tangle the code into a mess.... the goal of refactoring is to clean up code!

Member

retailcoder commented Jan 1, 2016

Yes, the extracted members should be removed from their original location, and call sites rewritten so that the code still compiles and runs just like before the refactoring.

If an extracted method has a dependency on a member that isn't selected, that member should be automatically selected - you don't want to introduce circular dependencies and tangle the code into a mess.... the goal of refactoring is to clean up code!

@Hosch250

This comment has been minimized.

Show comment
Hide comment
@Hosch250

Hosch250 Jan 2, 2016

Member

This could quickly become a mess. Selecting enough members will move the entire class over. One solution could be to copy unselected dependencies and move selected dependencies. This is one of those issues that should be easily customizable because these situations will likely need heavy user input and some non-duck modification. Perhaps we should have options to move dependencies, copy dependencies, or do nothing. If the user knows there is a closed loop, they can choose to move them. Most use cases will likely need copy, but many will just need slight modification after moving, and the user can select to just move them.

Member

Hosch250 commented Jan 2, 2016

This could quickly become a mess. Selecting enough members will move the entire class over. One solution could be to copy unselected dependencies and move selected dependencies. This is one of those issues that should be easily customizable because these situations will likely need heavy user input and some non-duck modification. Perhaps we should have options to move dependencies, copy dependencies, or do nothing. If the user knows there is a closed loop, they can choose to move them. Most use cases will likely need copy, but many will just need slight modification after moving, and the user can select to just move them.

@retailcoder

This comment has been minimized.

Show comment
Hide comment
@retailcoder

retailcoder Jan 2, 2016

Member

I'd like the thing to work just like the way ReSharper has it.. ideally :--)

Member

retailcoder commented Jan 2, 2016

I'd like the thing to work just like the way ReSharper has it.. ideally :--)

@Hosch250

This comment has been minimized.

Show comment
Hide comment
@Hosch250

Hosch250 Jan 2, 2016

Member

I looked at R#'s version, and it sounds like it is essentially my suggestion, with a few other refinements. However, my classes are beginning to go up, so I am not sure if I will be able to complete this in time. Feel free to take over where I have left off if I do not complete it.

Member

Hosch250 commented Jan 2, 2016

I looked at R#'s version, and it sounds like it is essentially my suggestion, with a few other refinements. However, my classes are beginning to go up, so I am not sure if I will be able to complete this in time. Feel free to take over where I have left off if I do not complete it.

@retailcoder

This comment has been minimized.

Show comment
Hide comment
@retailcoder

retailcoder Jan 3, 2016

Member

I'm starting to think this one might be a little too complex and time-consuming to release with 2.0.. how about we keep it for a later release, when the dialogs are in XAML? The UI will be easier to get right, and we'll have time to tweak the parser API to intake a modified token stream.

Member

retailcoder commented Jan 3, 2016

I'm starting to think this one might be a little too complex and time-consuming to release with 2.0.. how about we keep it for a later release, when the dialogs are in XAML? The UI will be easier to get right, and we'll have time to tweak the parser API to intake a modified token stream.

@Hosch250

This comment has been minimized.

Show comment
Hide comment
@Hosch250

Hosch250 Jan 4, 2016

Member

Fine by me. I'm thinking I want to take this week pretty much off before school gets in full swing again anyway. I won't delete my branch, though. I've done some good work here.

Member

Hosch250 commented Jan 4, 2016

Fine by me. I'm thinking I want to take this week pretty much off before school gets in full swing again anyway. I won't delete my branch, though. I've done some good work here.

@Hosch250 Hosch250 modified the milestones: v2.1, Version 2.0 Jan 18, 2016

@retailcoder retailcoder removed this from the Version 2.1.x milestone Apr 12, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment