Review Request: Change KCalendarSystem to a Shared D Pointer model

John Layt johnlayt at googlemail.com
Mon Jan 11 17:34:26 GMT 2010


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/2557/
-----------------------------------------------------------

Review request for kdelibs.


Summary
-------

KCalendarSystem is an abstract virtual base class with each calendar system re-implementing virtual methods as required.  Some new features will require new virtual methods to be implemented, but KCalendarSystem has no reserved virtual table slots, doesn't inherit from QObject, and has no virtual_hook.  The only sensible option left is to use the Shared D Pointer method as outlined at http://techbase.kde.org/Policies/Library_Code_Policy#Shared_D-Pointers and have the new 'virtual' methods be implemented in the derived private class.

There are two non-standard steps involved in doing this that are the main aim of this review:

1) The existing base class d is Private, and BC prevents it changing to Protected or adding a new Protected d_ptr.  Instead all the implementation classes are made Private friends of the base class.  As the d is the only private part of the base class I don't see this as an issue.

2) The existing derived implementation classes all have existing d pointer Private classes.  Rather than create a second Private class for each implementation, I have simply derived each of the existing Private classes from the base Private class.  I don't think this will break BC.

This is illustrated in the diff using the base KCalendarSystem class and the derived KCalendarSystemHijri class (Other files have been excluded from the diff for clarity as it's the same pattern repeated).

Note that the actual calendar implementations are all private, i.e. not exported and headers not installed, only base class KCalendarSystem has ever been exposed.


Diffs
-----

  trunk/KDE/kdelibs/kdecore/date/kcalendarsystemhijri.cpp 1072250 
  trunk/KDE/kdelibs/kdecore/date/kcalendarsystemhijri_p.h 1072250 
  trunk/KDE/kdelibs/kdecore/date/kcalendarsystemprivate_p.h PRE-CREATION 
  trunk/KDE/kdelibs/kdecore/date/kcalendarsystem.h 1072250 
  trunk/KDE/kdelibs/kdecore/date/kcalendarsystem.cpp 1072250 

Diff: http://reviewboard.kde.org/r/2557/diff


Testing
-------

Standard unit tests still pass.


Thanks,

John





More information about the kde-core-devel mailing list