| T O P I C R E V I E W |
| andre.horst |
Posted - Aug 06 2007 : 07:31:21 AM
If implementing a new interface via inheritancce of a general class or pure virtual class, all virtual methods should be taken over to the sub-class if the method-declarition is not present. |
| 13 L A T E S T R E P L I E S (Newest First) |
| sean |
Posted - Nov 18 2013 : 3:26:54 PM
Change Signature was overhauled in build 2007 and now supports changes to a class hierarchy. |
| support |
Posted - Jan 24 2011 : 4:04:22 PM
case=1505 is implemented in build 1840 |
| feline |
Posted - Aug 10 2007 : 5:17:12 PM
I have added these rules as a note to case=8117, they do seem like a very good way forward.
I was rather proud of that data mapping code, a very effective mix of an abstract base class with default functions and template wrappers - I think it was the only time I ever wrote a template function while I had that job  The high light in the middle of a very nasty, very messy project.
From memory there were about 16 derived classes, and most of them used the defaults, while a couple of them looked nothing like the base class (totally different data types and lookup rules) but ran off the same basic interface.
Perhaps not the most elegant solution, but it felt nice to do something a little different with C++ for a change |
| sl@sh |
Posted - Aug 10 2007 : 05:24:38 AM
Sounds gr8!
Btw., I knew it is legal to make a declaration abstract and still provide an implementation, I just didn't realize how this could be useful. Your example makes sense, although, as you point out, your purpose could be solved in another way. (But, of course, it isn't the task of VA to point that out!  ) |
| feline |
Posted - Aug 09 2007 : 10:39:48 AM
An initial answer, this compiles:
class CDataFile {
public:
virtual char *getFileName() = 0 { return "default if you can use it"; }
};
class CTextFile : public CDataFile {
virtual char *getFileName() { return CDataFile::getFileName(); }
};
and I have actually written code like this. On one occasion I needed a load of data mapping classes. Most of them were built out of a simple map / dictionary class, with various custom lookup rules, but some used more complex storage. So I used abstract functions with default implementations for the map / dictionary storage, so they could be used if suitable, but you had to manually choose to use them. I didn't want any of the derived classes using the defaults without thinking about it carefully first.
Of course later on I got clever and started using a template function to wrap all of these classes, since they had the same interface, but different types
Nothing to do with VA in case you are wondering, but it was a very good solution for a very specific problem
Given this happens though I am wondering what is the "best" simple action VA can perform, that will be fine 99% of the time.
I am leaning towards the single refactoring called something like "Override in derived classes".
* Trigger on a function. It will make the function in the base class virtual if it is not already virtual.
* It will add a virtual declaration only to all selected derived classes.
* If the function in the base class was abstract "ignore" that, none of the derived functions will be abstract. This is a reasonable default state, and we really don't want to ask for every derived class.
There is an argument that you should not make the function virtual in the final child class (given in one of the Effective C++ books from memory) but for now I feel we are better off making all new function declarations virtual. A simple and well defined behaviour, and generally reasonable.
I agree, if this is done people will want to run it on several functions at once, but lets see about getting this operation added at all first |
| sl@sh |
Posted - Aug 09 2007 : 03:52:54 AM
VA has the policy of doing the least possible steps needed by most of the users in a given situation. The two extremes you have given are certainly legal and at least for the second one there might actually be a sensible use (I've done that myself to restrict visibility of a particular function in a tree of derived classes). However, I expect both cases to be rare ones.
The first example actually is an example of overwriting a method instead of overriding which is bad style if not outright semantically wrong, depending on the context. I would advise to not have VA support this sort of coding, especially if you decide to go by the name I suggested above, 'Create Override'. This refactoring method should only work on virtual operations.
The last example you gave actually is more like what I was thinking of: a tree of derived classes that at one or possibly more levels may want to implement an operation declared (pure) virtual on some base class. The only way I see to properly cover this would be a dialog prompting the user to check all the derived classes that he wants to create overrides for.
Whether the overridden method should be labelled virtual is a good question. Normally it should stay, so classes derived from the target class at a later point could themselves implement another override without the need for other changes. Eliminating the 'virtual' keyword on the override basically makes the implementation 'final'. I'd say since in most cases there is no need for an overridden implementation to be declared final, the 'virtual' should stay, leaving it to the user to delete it as needed.
Declaring an override 'pure' ( adding "=0" ) is another case altogether. There are few possible reasons why anyone would want to create an abstract override. the ones I could think of would be:
- change of visibility
- change of return type
- force an override for methods that already have an implementation in a base class
All of these I consider rare cases, especially since, technically, the same things could be achieved within the concrete derived classes (although if there is more than one, this might result in redundant coding). These are the kind of things I would expect a system designer to put into a project for the purpose of enforcing certain programming standards (I've actually seen people doing it for exactly this reason). I'd say these people could live with VA removing the 'abstract' property from newly created overrides as a standard behaviour.
That said, once you implement this, I can already see the next item on the wish list: create overrides for a set of virtual methods all at once.
P.S.: I was just thinking of something:
1. overriding a virtual function in a derived class usally implies creating both a declaration and an implementation. In that case, the declaration obviously may nor be abstract ("=0")
2. if "Create Implementation" is called on a method declaration labelled abstract, this doesen't make sense - however, if "Create Implementation" also simply checks for the 'abstract' property and removes it as needed, this would be helpful.
3. Combining the two - if the suggested refactoring method "Create Override" just created declaration overrides, it could make these declarations all abstract. The user could then call "Create Implementation" on each and thus remove the 'abstract property whereever needed.
4. It would be even better if there were two refactoring methods for overriding: "Create override declaration" and "Create override implementation". The first would only create abstract declarations as suggested above, the second would create concrete declarations and implementations to go with them. |
| feline |
Posted - Aug 08 2007 : 2:21:29 PM
virtual... an interesting thought is occurring to me. At one extreme you have the code:
class testBaseClass
{
char *GetClassName() const;
};
class testChildClass : public testBaseClass
{
char *GetClassName() const;
};
while at another extreme you have the code:
class testBaseClass
{
virtual char *GetClassName() const = 0;
};
class testChildClass : public testBaseClass
{
virtual char *GetClassName() const = 0;
};
On reflection it is not VA's job to decide which combination of virtual, abastract, and other keywords you want to apply to your function.
A more interesting thought is the case of three class:
class CResult { virtual int getResultSize() const = 0; };
class CTextResult : public CResult { };
class CPlainTextResult : public CTextResult { };
where the user wants the function to be abstract in CTextResult but concrete in CPlainTextResult.
This leaves me thinking the best answer is for VA to simply "copy and paste" the function declaration in the base class to all selected derived classes, and then let the user make any changes they want later on. |
| sl@sh |
Posted - Aug 08 2007 : 10:17:05 AM
As mentioned in my P.S. above, a modification of Add Member won't be needed if a new refactoring method 'Create Override' is being introduced. Once you get that you can simply add a virtual function in the base class using Add Member, then use 'Create Override' on that function to create declarations and implementations in derived classes.
Maybe this refactoring could be made to work even on non-virtual functions, adding the virtual keyword to the base function in the process. This would cover the case you describe. (I wouldn't consider this a primary goal though, adding 'virtual' to a function doesn't require that much work to start with ) |
| feline |
Posted - Aug 08 2007 : 08:27:35 AM
I am a little wary of adding a step or option to the current Add Member / Add Similar Member refactorings. The current design is to keep things nice and simple.
In my experience with class hierarchies I have myself making existing functions in the base class virtual, when I discovered the need to override the function in a specific edge case in one of the specialised child classes. Then again the code base where I was doing this was a little "odd" and full of edge cases.
I have put in a feature request for this general idea, to see what the developers make of it. Personally I like the idea, and it seems like it could be very helpful:
case=8117 |
| sl@sh |
Posted - Aug 08 2007 : 04:21:42 AM
Anything that refers to attributes, not operations, should not be affected. So, Encapsulate Field is definitely off the list.
Create Implementation is not concerned either - when you want to create an implementation for a specific function, you normally do that at one specific level of your inheritance tree. Having various implementations that override each other is a possibility, but that IME is the exception rather than the rule.
Create Declaration - it would make sense to have this affect derived classes only if the declaration is going to be virtual. However, since you call that refactoring method from an existing implementation, that would only make sense when you intend to override said implementation elsewhere. As mentioned above, I'd consider this a rare case, so it shouldn't be taken into account.
Add Member / Add Similar Member - these would indeed make sense, provided a virtual operation is being added. I wonder whether the existing functions should include an additional step to serve that purpose - which would inhibit their use in the currently implemented sense, or if new refactoring methods should be created to better suit the steps needed to define a set of virtual and derived methods.
P.S.:
Creating an implementation along with a declaration for a specific virtual operation of a base class in one more derived classes is something no existing refactoring method currently offers. (creating only a declaration override has it's uses as well, but I consider that a rare exception, so most of the time you would want to create both implementation and declaration). If you consider introducing such a function, there would be no need to modify Add Member / Add Similar Member! |
| feline |
Posted - Aug 07 2007 : 07:52:47 AM
You make some interesting points.
Find References on a function in the base class currently scans for references in the derived classes, and Rename also does the same.
Extending Change Signature to do the same is quite reasonable, I have put in a feature request for this:
case=8095
The idea of adding a function to the derived classes... Arguably this effects the following refactoring operations:
Add Member
Add Similar Member to <class name>
Create Declaration
Create Implementation
Encapsulate Field
"Create Implementation" is a bit debatable here, and the existing problem that Create Implementation does not currently check for an existing implementation before making a new one would get a lot worse with a complex class hierarchy. Of course the same problem, in reverse, applies to Create Declaration.
Encapsulate Field is definitely debatable, and on reflection I am inclined to say it should not effect the derived classes.
I am a little concerned about the scope of this at the moment
I am wondering if instead a new refactoring called something like "Override in derived class" is the solution. You could then trigger this on a class member function, get some form of dialog that lists all of the derived classes VA can find, you select the ones you want, and a declaration is added to each of those classes. |
| sl@sh |
Posted - Aug 06 2007 : 11:15:40 AM
Actually I am not so much concerned with (auto-)creating implementations for virtual base class methods in new classes, more with adding, updating or removing such implementations over a period of time. Creating the implementations initially is only a matter of copy and paste, mostly, but keeping them up to date is much more tedious.
When I work on a set of classes, most often it is a single method (or at least a very limited number of related methods) that I need to add, change, or move to another class within the inheritance tree. As a result I need to update all the derived classes' implementations.
Hence, what I'd like are refactoring methods that work on virtual function declarations, not derived classes! Change Signature for instance would be a lot more useful if it changed implementations within derived classes as well.
Also any (new) virtual method could have a refactoring entry to copy a declaration (and implementation) to derived classes, but that would require a dialog to select from all derived classes that should implement it. I realize this would be harder to implement, but certainly doable.
I do realize that applying any refactoring methods over inheritance trees is bound to cause a lot of headaches regarding edge cases. But I am also convinced that once you have a reliable mechanism of traversing those trees implemented, it could be very helpful. |
| feline |
Posted - Aug 06 2007 : 08:30:10 AM
We are considering offering something like this:
case=1505
However there are times when you do not want to implement all virtual methods, so we probably need to ask the user which ones they want to implement. |
|
|
