[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-core-devel
Subject:    Review Request: Change KCalendarSystem to a Shared D Pointer model
From:       "John Layt" <johnlayt () googlemail ! com>
Date:       2010-01-11 17:34:26
Message-ID: 20100111173426.23597.68450 () localhost
[Download RAW message or body]


-----------------------------------------------------------
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


[prev in list] [next in list] [prev in thread] [next in thread]