RE: FW: Enabling sharing of Qwt objects - https://sourceforge.net/p/qwt/patches/62/
Brought to you by:
rathmann
From: Peter Myerscough-J. <pet...@ma...> - 2017-02-03 12:15:46
|
> Hi Peter, > > > (In the aim of C++ interface arguments and knowing you are > happy to > make ABI/API changes going to 6.2) > Well "happy" is not exactly the right word. > > Changing APIs with 6.2 is possible and has already been done. But it does not > mean being careless about it. I hope I wasn't being careless, and I am aware I am not a maintainer of a really nice library. > But removing the virtual keyword breaks applications ( maybe not being > maintained anymore ) in a nasty way, where the compiler does not even tell > you ... May be there is a way to ensure non-breakage for old applications, but removing virtual from functions is only a tidying step. If base interface classes were introduced, then these could be limited to the functions Qwt uses/requires when it uses those classes, and the original implementation could maintain its old virtual functions, not breaking old code. The isValid functions could then be implemented in the symbols. I think adding a virtual function to make the interfaces complete is acceptable, and I don't think it will break the classes or derived classes. I have tried to think through the scenarios, and I don't think making a function virtual is bad or has significant side effects. (It may be worth adding Q_DECL_OVERRIDE to overridden virtual methods in the Qwt derived classes too.) > ( the same for QwtRasterData::setInterval that also shouldn't be virtual ) > > > ...and the pallete's are sometimes not used by the abstract class, but > > are used by the derived implementations.) > > That is totally fine with me as it expresses the assumption of any compass > rose being made of a palette. This assumption might be wrong, but this is not > a problem of the implementation made from it. > > > Essentially I would propose forming (abstract) interfaces for these > > classes and making Qwt classes that use these classes accept pointers > > to these interfaces. This would ensure that inheritance is cleanly > > possible and that the functions used by Qwt with classes that are to > > be inherited from are complete and minimal. > > ( Well this not really about making objects shareable anymore ) Actually, making the changes to the listed classes would make them all shareable using a user made proxy class, my early sharing use-case. I think more importantly they define the interface that Qwt uses when interacting with these classes, and particularly these classes because they are expected to be derived from. This clarifies which functions must be implemented by derived classes for Qwt to use them. The symbol classes both have a non-virtual style function that Qwt calls to determine if the symbol should be used, but if an implementation wishes to dynamically/reactively check if the symbol is to be used it cannot, it must actively maintain the state of this variable because it is not able to respond when Qwt requests it. Similarly the QwtRasterData object's interval could also be a dynamic property. F.e. if the raster data is used to contain a QAbstractItemModel, then the interval could be a property that is dynamically calculated, e.g. return QwtInterval(startTime, startTime + rowPeriod*model->rowCount());. > In general having an minimal abstract interface definition for all and > everything leads to heavy casting at places, where you have to deal with the > real thing - usually in application code. > > Considering, that compiling without RTTI support is a feature of Qt and we > are talking about classes, where you can't use qobject_cast there is not even > a safe way for casting. In which case the application writer would be able to pass in derived classes and the compiler would automatically cast them to the base interface. The other way around, the application writer already knows they have to be careful, and would probably resolve to some custom RTTI convention like Qt does, or make an application rule that this type is the only type for assigning to X. There are already QwtColorMap classes that are derived and passed by the base class pointer, but there is a type() function to enable simple custom RTTI behaviour. The symbols both have a similar property ("style"), but for them it is encoded both as if the symbol is valid and if it is overloaded. QwtRasterData, which must be overloaded to be used, has not such capability, unless you consider abusing the setAttribute function. I think therefore that this argument is not applied consistently across the library for classes that are to be overloaded. This is not to say it shouldn't, but currently it isn't. I don't think this should be a blocking issue to defining minimal complete interface classes. > F.e I changed setCanvas/canvas of QwtPlot into using a QWidget. This makes > it possible to pass in a opengl based widget, but ... > Uwe > > ------------------------------------------------------------------------------ > Check out the vibrant tech community on one of the world's most engaging > tech sites, SlashDot.org! http://sdm.link/slashdot > _______________________________________________ > qwt-interest mailing list > qwt...@li... > https://lists.sourceforge.net/lists/listinfo/qwt-interest |