Hi,
I've said this also in the reviews, but perhaps it got lost: My primary concern with this API is that it promotes an unclear ownership model. Typically we have API that is either a sub-class of QObject (where the parent may delete the children, such as QFile) or we have value based classes (such as QDateTime). There is also a slightly less common, third model with explicitly shared types, I think mostly acting as singletons. With QAbstractCalendar there's an entirely new model (which also includes a clone() method). I think that model should reconsidered, but if not, then it needs to be documented and it needs to be clarified how this model applies to the exposure of calendars to QML. Otherwise you easily risk creating dangling pointers (as pointed out in the QML review). A secondary concern is that the API, once submitted, is not extensible. We cannot add new properties without breaking binary compatibility, due to the use of virtual functions in the frontend. I think in the past we've solved this by having a nice front-end API with non-virtual functions (those we can add) and channeling this through to a backend that uses fewer virtual functions but enums (that can be extended) to dispatch. Another alternative is the extension mechanism that we've used with QFile/QIODevice. Simon ________________________________ From: Development <[email protected]> on behalf of Edward Welbourne <[email protected]> Sent: Tuesday, July 31, 2018 12:10:11 PM To: [email protected] Subject: [Development] Calendar classes: API review request Feature freeze is drawing near, as Jani just reminded us, and Soroush's work on calendar support is in a state that looks (to me) ready to use. Please would those with strong views on API design take a look at [0] and speak up now, so that we don't end up reworking it all during the API review phase of the release. * [0] https://codereview.qt-project.org/182341 This would also be a good time to take a look at Hamed's related work in QML: * https://codereview.qt-project.org/228407 * https://codereview.qt-project.org/188184 Eddy. _______________________________________________ Development mailing list [email protected] http://lists.qt-project.org/mailman/listinfo/development
_______________________________________________ Development mailing list [email protected] http://lists.qt-project.org/mailman/listinfo/development
