[zypp-devel] Re: question about Media virtuals
On Tuesday 24 February 2009 18:49:21 you wrote:
question :-)
MediaHandler has virtuals, and provide default implementation.
All concrete handlers override them, foo, only to call MediaHandler::foo() in the reimplementation. What is the purpose of this?
Depends on the design of the base class. This is usually the case if the base class defines all pure virtual: virtual int pure() = 0; You usually do this to enforce each derived class to check and explicitly state that the provided default implementation is suitable. If you add a new pure virtual, or change it's signature, all classes must be visited and checked.
(at least, makes really hard and boring to do any refactoring changing method signatures).
That's the intent. If you add a method or change the signature the compiler will tell you all locations you have to touch and review. That's esp. helpfull if not all developers are that familiar with the code. We e.g failed to do this in the callback classes, and sometimes pay the price. It happens someone changes a callback signature, and no one recognizes it, because the compiler does not complain (per default not even warns). needs: -Woverloaded-virtual (-Wnon-virtual-dtor) If the callback methods were pure, it would not have happened that e.g. pkg-kit listens to the wrong callback for more than a year. That would have saved investgation time time and several bugreports. Just for the convenience of 'easy refactoring'. Now as we have automated builds, thus recognize early if something does not compile, I'd tend to introduce more pure virtuals instead of hoping all others turn on the proper compiler warning and also read them. Your experience may be 'hard and boring', but (no only) if you have to fix things under preassure, you will appreciate the compiler assistance. While refactoring the '= 0' hinders. Thats why you could do: - Create some header e.g. 'base/virtuals.h' #ifdef REFACTORING #warning PURE VIRTUALS DIABLED WHILE REFACTORING #define PUREVIRTUAL #else #define PUREVIRTUAL =0 #endif - Include it an all header files that may use pure vitrtuals: #include "zypp/base/virtuals.h" ... virtual int pure() PUREVIRTUAL; - And configure to compile with -DREFACTORING -- cu, Michael Andres +------------------------------------------------------------------+ Key fingerprint = 2DFA 5D73 18B1 E7EF A862 27AC 3FB8 9E3A 27C6 B0E4 +------------------------------------------------------------------+ Michael Andres YaST Development ma@novell.com SUSE LINUX Products GmbH, GF: Markus Rex, HRB 16746 (AG Nuernberg) Maxfeldstrasse 5, D-90409 Nuernberg, Germany, ++49 (0)911 - 740 53-0 +------------------------------------------------------------------+ -- To unsubscribe, e-mail: zypp-devel+unsubscribe@opensuse.org For additional commands, e-mail: zypp-devel+help@opensuse.org
Michael Andres wrote:
On Tuesday 24 February 2009 18:49:21 you wrote: Depends on the design of the base class.
This is usually the case if the base class defines all pure virtual:
virtual int pure() = 0;
I am not talking about pure virtuals here. MediaHandler::getFile is a normal virtual, providing a base implementation. All derived handlers override this functions. 99% of them just to call MediaHandler::getFile, the only ones that actually overrides it is curl, who has a different implementation. Why the other handles just don't override it? Duncan -- To unsubscribe, e-mail: zypp-devel+unsubscribe@opensuse.org For additional commands, e-mail: zypp-devel+help@opensuse.org
On Wednesday 25 February 2009 12:02:33 Duncan Mac-Vicar P. wrote:
Michael Andres wrote:
On Tuesday 24 February 2009 18:49:21 you wrote: Depends on the design of the base class.
This is usually the case if the base class defines all pure virtual:
virtual int pure() = 0;
I am not talking about pure virtuals here. MediaHandler::getFile is a normal virtual, providing a base implementation. All derived handlers override this functions. 99% of them just to call MediaHandler::getFile, the only ones that actually overrides it
Then you have to ask the author. Maybe it was pure at the beginning and lost it. Or the author saw the implementation of other pure methods and did it the same way without thinking about it. Or it was forgotten to make it pure.... BTW in code 11 it is pure: virtual void getFile( const Pathname & filename ) const = 0; -- cu, Michael Andres +------------------------------------------------------------------+ Key fingerprint = 2DFA 5D73 18B1 E7EF A862 27AC 3FB8 9E3A 27C6 B0E4 +------------------------------------------------------------------+ Michael Andres YaST Development ma@novell.com SUSE LINUX Products GmbH, GF: Markus Rex, HRB 16746 (AG Nuernberg) Maxfeldstrasse 5, D-90409 Nuernberg, Germany, ++49 (0)911 - 740 53-0 +------------------------------------------------------------------+ -- To unsubscribe, e-mail: zypp-devel+unsubscribe@opensuse.org For additional commands, e-mail: zypp-devel+help@opensuse.org
participants (2)
-
Duncan Mac-Vicar P.
-
Michael Andres