libs/kworkspace/kdisplaymanager.cpp mess

[Aaron J. Seigo]

On Thursday, April 25, 2013 15:11:25 Martin Briza wrote:
> I would leave creating the CK module to somebody who is experienced with
> how exactly the whole system works and knows if it is safe.

if it is a refactor, this should be a matter of moving the CK code as-is into a new file. if the refactor 
requires significant changes to the existing design of class in terms of how it works, then it's 
probably not a refactoring :)

>   - The KDisplayManager class is used only on a few places so replacing its
> constructions with calls to the factory will be easy.

i don't think KDisplayManager's public API needs to be changed in any way.

what would be nice is to have an abstract base class and have a subclass for each of the session 
management methods. then in KDisplayManager's ctor it can decide which is the correct backend 
to implement.

right now the pattern is sth like:

void KDisplayManager::someAction()
{
     switch (DMType) {
          case NewKDM: 
                ... some kdm specific code ...
          case NewGDM:
                ... some gdm specific code ...
     }
}

very non-object oriented, but made sense given what it started out. now .. as you noticed .. not so 
much. :) with an ABC that defines an interface containing all the actions (e.g. canShutdown, etc.) 
the pattern would then be sth like:

void KDisplayManager::someAction()
{
     d->backend->someAction();
}

and polymorphism would take care of having the right code be called.

each backend could go into its own file (though i imagine some backends will end up sharing 
code, cover multiple cases and/or subclass other backends). in the source tree, they could all go 
into a subdir of libs/kworkspace/ to keep it tidy.

i would probably also drop the oldKDM and oldGDM paths.

i assume you'll implement this in a feature branch to make discussing during devel easy?

-- 
Aaron J. Seigo
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20130430/28b0c2a4/attachment.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part.
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20130430/28b0c2a4/attachment.sig>