Author |
Topic |
|
legalize
Tomato Guru
USA
119 Posts |
Posted - Jul 31 2009 : 01:28:41 AM
|
1) Inline Method
We have Extract Method (Extract Function), but there's no way to do the reverse. All refactorings are inherently reversible and sometimes you need to inline methods that were improperly extracted so that you can re-extract them properly. There are other uses for inline method as well. Suppose you've introduced an overload of a method/function and leveraged the existing version by calling it from your new overload. Eventually, the code evolves to where the original overload is no longer used, except by the 2nd overload that leverages it. You'd like to inline the original overload into the second and eliminate the original version altogether.
Taking a cue from ReSharper, it would be great if Inline Method gave you the choice of inlining all instances of the method or only the instance containing the cursor. If all instances are inlined, it would be great if it also gave you the option to remove the declaration of the inlined method.
2) Narrow Scope of Declaration
When taking a monster long method and trying to extract out the chunks, it can be painful when the method has all its local variables declared at the top of the method instead of having them declared near their first use. It would be great to position the cursor on an identifier and have VAX move it to the most enclosing scope possible so that Extract Method doesn't end up with long parameter lists.
3) Split Local Variable
Similar to 2), when you have long monster methods they tend to reuse local variables making it difficult to extract methods on the chunks. The reuse tends to be along the lines of:
int foo = GetAFoo(); // use foo in chunk of code
foo = GetAFoo(); // use foo in chunk of code
... and so-on
Split Local Variable introduces a new variable at the next assignment point:
int foo = GetAFoo(); // use foo in chunk of code
int foo2 = GetAFoo(); // use foo2 in chunk of code
All references to the original variable are changed to the new variable from the point of the new variable's introduction until the original variable goes out of scope. This refactoring should only apply when you can determine for certain that the old value is lost at the point where the new variable declaration is inserted.
4) Extract Interface
It would be great to be able to select a class and extract an interface for that class by entering the name of the interface and selecting methods from the class for inclusion in the interface. (They would be pure virtual members and the extracted interface would have a virtual destructor that is empty.) |
http://legalizeadulthood.wordpress.com |
Edited by - legalize on Jul 31 2009 10:02:32 AM |
|
mwb1100
Ketchup Master
82 Posts |
Posted - Jul 31 2009 : 12:00:10 PM
|
Just a comment: "Inline Method" might need to have a different name, since C++ actually has 'inline' for methods/functions, the name might be somewhat confusing. You seem to be talking about actually placing the code for the method at the point of the the current call, rather than simply moving the method implementation into the header and marking it as 'inline'.
|
Edited by - mwb1100 on Jul 31 2009 8:31:09 PM |
|
|
legalize
Tomato Guru
USA
119 Posts |
|
feline
Whole Tomato Software
United Kingdom
18951 Posts |
Posted - Aug 03 2009 : 12:16:53 PM
|
We are considering Inline Method:
case=1493
Narrow Scope of Declaration sounds interesting, and you make a good case for why someone would want to do this. I have put in a feature request to see what our developers make of this:
case=30486
Split Local Variable is another interesting idea. I have put in a feature request for this, to see what our developers make of it:
case=30487
Extract Interface, maybe I am missing something here. This almost sounds to "simple". That is not really the right word, but I am not sure how to describe my thought.
Is this something you often do? I think I have only done this a handful of times, in all my years. It sounds a lot like just copy / paste the class declaration, which is easy to do with VA Outline, where you can pick out just the bits you want, and then just rename the copy and add "= 0" to the end of each of the declarations.
I see the appeal, I am just not sure how useful this would really be. |
zen is the art of being at one with the two'ness |
|
|
legalize
Tomato Guru
USA
119 Posts |
Posted - Aug 04 2009 : 12:18:52 AM
|
quote: Originally posted by feline Extract Interface, maybe I am missing something here. This almost sounds to "simple". That is not really the right word, but I am not sure how to describe my thought.
Is this something you often do? I think I have only done this a handful of times, in all my years. It sounds a lot like just copy / paste the class declaration, which is easy to do with VA Outline, where you can pick out just the bits you want, and then just rename the copy and add "= 0" to the end of each of the declarations.
I see the appeal, I am just not sure how useful this would really be.
You need to mix with the test-driven development crowd more :-).
I wrote a series of blog posts covering how to use Boost.Test to do test-driven development in C++. Part 4 shows how to do TDD with GUI code: http://legalizeadulthood.wordpress.com/2009/07/05/c-unit-tests-with-boost-test-part-4/
The reason I bring this up is that interfaces are essential to test-driven development and being able to test classes in isolation from the rest of the system. In that blog post, I describe how we create an interface that encapsulates the state of a dialog, even before we have an actual concrete dialog class. We put the behavior of the dialog in a mediator class that orchestrates the interactions between the controls on the dialog. The mediator is a concrete class, but it collaborates with an abstract dialog. We use a test double as a stand-in for the concrete dialog while we are testing the mediator.
The history of C++ is such that it has a tendency to create piles of concrete classes that are highly coupled to each other. However, this doesn't lead to flexible software because the implementation details of one class have a tendency to leak over into the other classes with which they collaborate. The answer to this coupling problem is to have concrete classes collaborate with interfaces ("dependency inversion principle", details depend on abstractions http://en.wikipedia.org/wiki/Dependency_inversion_principle). We are then free to change the implementations of those interfaces without worry that another concrete class will be coupled to the changes in the implementation.
All this stuff has been known in OOP circles for a long time, but for some reason its new to a lot of C++ programmers. I guess because they have been using C++ as "a better C" and using inheritance only as a reuse mechanism instead of using inheritance to say "this class implements this interface". The COM world has picked up on interfaces, although it has other issues because it also tries to be language-agnostic and provide location transparency with respect to execution (i.e. you never know if you're talking to the "real" object or just a local proxy for a remote object).
Once you start getting into test-driven development, polymorphism through interfaces becomes the best way to get your class under test isolated from the other classes so that you know the bug is in this class under test if the test fails. There are alternative means to achieving the same ends (protected virtual methods with a test double derived from the production class and overriding the methods, preprocessor [yuck!], templates and probably some others), but they are more clumsy and difficult to use properly while still keeping the test code and the production code separated.
OK, so that explains my motivation for wanting interfaces, but the example I described in the blog post created the interface first before we had a concrete class, right? So why would I need Extract Interface? Well, if I'm taking legacy code (see http://legalizeadulthood.wordpress.com/2007/04/11/working-effectively-with-legacy-code-by-michael-c-feathers/), by which I mean code without unit tests, and I need to enhance/change it in some way, then first I want to fit tests around it before I change it. That way I know that I haven't broken any behavior I want to keep working before and after my change, and I can use TDD for the change I want to make. However, my existing class is going to be collaborating with other classes. Those classes shouldn't be part of my tests on my existing class. I need a standin test double for those other classes. The best way to do that is to Extract Interface on those other classes and have my class I want to modify collaborate with those other classes through the interface instead of through the concrete implementation.
So yes, Extract Interface is extremely handy for this style of programming.
Oh, and Extract Interface does more than just create a new interface. The class from which the interface is extracted is made to inherit from the newly extracted interface. As you can see from the discussion above, the point is not merely to create an interface that is just out there all by itself. The point is to extract an interface from an existing class and have that existing class also implement the newly extracted interface. Then I can have a standin test double for the class.
See http://www.refactoring.com/catalog/extractInterface.html for a UML diagram of the Extract Interface process.
If you haven't yet read "Refactoring: Improving the Design of Existing Code" by Martin Fowler, I recommend you get a copy posthaste and start reading it. |
http://legalizeadulthood.wordpress.com |
Edited by - legalize on Aug 04 2009 12:21:49 AM |
|
|
feline
Whole Tomato Software
United Kingdom
18951 Posts |
Posted - Aug 04 2009 : 2:16:22 PM
|
Thank you, this explains why this is helpful, and why you want to do this quite often. I have put in a feature request, referencing this discussion:
case=30546 |
zen is the art of being at one with the two'ness |
|
|
legalize
Tomato Guru
USA
119 Posts |
Posted - Aug 05 2009 : 12:57:42 AM
|
While we're talking about Extract Interface, I can see some options that would make it more widely applicable: - select which methods on the class are extracted into the interface - specify whether or not the class from which the interface is extracted is modified to implement the interface - If no, then give an option to generate an adapter for that concrete class to the interface
The reason I mention that last bit is that sometimes you want to extract interface for a collaborator, but you don't control the collaborator (think MFC or any other GUI framework where the controls/forms don't use interfaces).
The adapter is simple boiler-plate that contains an instance of the adapted class and derives from the interface. The implementation of the interface delegates everything to the adapted class. |
http://legalizeadulthood.wordpress.com |
|
|
feline
Whole Tomato Software
United Kingdom
18951 Posts |
Posted - Aug 05 2009 : 1:53:25 PM
|
Point 1, in my feature request I have said that I hope to see a dialog box showing a list of member functions with check boxes next to them, so you can select which member functions become part of the interface, and which do not.
Points 2 and 3, I don't understand. Consider this following very simple C++ example. Before we have:
class testExtractInterface
{
int getHeight() { return 1; }
int getWidth() { return 1; }
void setName(const char *pName) { ; }
};
and afterwards you might have:
class newInterface
{
virtual int getHeight() = 0;
virtual int getWidth() = 0;
};
class testExtractInterface : public newInterface
{
int getHeight() { return 1; }
int getWidth() { return 1; }
void setName(const char *pName) { ; }
};
since the interface is a function of the class you run the refactoring on, how can the original class be modified? The original class is now derived from the interface, but that is not really modifying it.
What do you mean by an adaptor? My understanding is that the interface just picks up certain members of the original class, so there is no need for any form of adaptor, since the class and the new interface class match exactly. |
zen is the art of being at one with the two'ness |
|
|
legalize
Tomato Guru
USA
119 Posts |
Posted - Aug 06 2009 : 08:35:20 AM
|
I would change it slightly so that the extracted interface as a virtual destructor with a default empty implementation:
class newInterface
{
virtual ~newInterface() { }
virtual int getHeight() = 0;
virtual int getWidth() = 0;
};
class testExtractInterface : public newInterface
{
int getHeight() { return 1; }
int getWidth() { return 1; }
void setName(const char *pName) { }
};
However, suppose that testExtractInterface isn't a class that I own, but its a class that comes from some 3rd party toolkit. In that case, the declaration for testExtractInterface is in a "stable" header and I shouldn't be modifying it. So I need a way to adapt testExtractInterface to newInterface:
class newInterfaceAdapter : public newInterface
{
public:
newInterfaceAdapter(testExtractInterface &target) : m_target(target)
{ }
int getHeight() { return m_target.getHeight(); }
int getWidth() { return m_target.getWidth(); }
void setName(const char *pName) { m_target.setName(pName); }
private:
testExtractInterface &m_target;
};
newInterfaceAdapter is a class that adapts testExtractInterface to newInterface. My code uses the interface and when I need to talk to testExtractInterface, I use newInterfaceAdapter instances to adapt testExtractInterface to newInterface.
Adapters also come up in TDD when I need to collaborate with another class, but that other class only provides static methods/functions. (MessageBox in Win32 is a great example of this.) If I create an interface collaborate with it, then my unit test can avoid creating a real MessageBox, which would prevent my tests from being automatic -- someone would have to click on the message boxes. Also, once I wrap MessageBox in an interface, I can control its interaction with my system under test by forcing my test double to pretend I've clicked OK or Cancel, depending on which code path I'm trying to test. |
http://legalizeadulthood.wordpress.com |
Edited by - legalize on Aug 06 2009 08:37:13 AM |
|
|
feline
Whole Tomato Software
United Kingdom
18951 Posts |
Posted - Aug 06 2009 : 4:07:38 PM
|
Adding the virtual destructor to the interface class makes sense, I have put a note on the feature request about this.
The adapter, I had not considered you trying to test classes from stable include directories, but your MessageBox example makes a lot of sense.
If I understand your example correctly this is a quite different refactoring operation. You are "wrapping" an existing class in this new Adapter. Would you expect to wrap all member functions of the class? Or would a dialog to select which members to wrap still make sense?
Also why is the adapter derived from newInterface? In general there is no "guarantee" of any relationship between the interface class "newInterface" and the 3rd party stable class "testExtractInterface".
I am wondering if I am missing something obvious here, or it could simply be that I am not really familiar with these techniques |
zen is the art of being at one with the two'ness |
|
|
legalize
Tomato Guru
USA
119 Posts |
Posted - Aug 07 2009 : 08:43:17 AM
|
OK, the idea behind Extract Interface is that I want to deal with that class through an abstraction and not a concrete relationship. The reason I want an interface around class B is because I'm trying to test class A that collaborates with B. I want A to talk to B through an interface so I can use a test double for class B while I'm testing A. Extract Interface is great for this when I control class B. When I don't control class B, I still need an interface for A to collaborate with so that I can fake out B during A's tests. So I still extract interface on B, but B can't be changed to inherit from the new interface so instead I need an adapter for B that adapts it to the extracted interface. My production code for A now collaborates with B through the interface by way of the adapter to the interface. My test code supplies test doubles for B through the interface.
Providing the adapter generation as part of Extract Interface is just a "nice to have", its not essential for Extract Interface. Adapters are simple delegation and I can write them out by hand if I wanted. However, because they are simple delegation and since the adapted class is the one from which I extracted the interface, the signatures of the interface methods and the adapted class are identical, its pure boilerplate and its a place where code generation tools could be productive. Still, its a nice to have, not a must have for Extract Interface. Get the basic refactoring of Extract Interface working first and then we'll see how often we need to make adapters.
There's more on Adapter pattern here in Wikipedia: http://en.wikipedia.org/wiki/Adapter_pattern but their example is more complex than what we're talking about here. |
http://legalizeadulthood.wordpress.com |
Edited by - legalize on Aug 07 2009 08:45:12 AM |
|
|
feline
Whole Tomato Software
United Kingdom
18951 Posts |
Posted - Aug 13 2009 : 1:53:40 PM
|
In my previous experience and reading about C++ I have seen some discussion about these adaptor classes. Since they are simply "empty" wrappers some people say they are pointless and get in the way, while other people offer up good reasons for using them.
I was not familiar with using them like this in automated testing, but I do see the appeal. I suppose the real question is how often you want / need a refactoring command like this. |
zen is the art of being at one with the two'ness |
|
|
legalize
Tomato Guru
USA
119 Posts |
Posted - Aug 14 2009 : 12:09:33 PM
|
It doesn't come up as often as the need for Extract Interface. When it does come up, they are not difficult to create by hand, just tedious. It can be bothersome for classes with very large numbers of methods, but those aren't good classes anyway. |
http://legalizeadulthood.wordpress.com |
|
|
|
Topic |
|
|
|