On Tue, 30 Apr 2013 17:33:57 +0200, Aaron J. Seigo <ase...@kde.org> wrote:
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 :)
I'm still talking about refactoring. I'm a bit afraid of moving the CK
code though as I'm not 100% sure all the functionality needed is actually
there and would work as it does now. That's the reason I'm proposing the
change with temporarily leaving CK support in the state as it is now.
To avoid misunderstanding (I'm quite convinced I'm not able to make myself
clear enough due to my English): this doesn't break anything nor introduce
any kind of regression, the functionality stays the same.
- 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.
The API itself wouldn't change. I'm just proposing usage of a singleton
factory class to have only one instance of the KDisplayManager class
instead of constructing the object all over. But say the word and I
implement it as you suggested below.
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.
Yes, I totally agree with the abstract base class idea, though I still
think having a private subclass is a bit messy practice. Yet once again,
I'm here to listen and learn, so the final word on this topic is up to you.
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.
Yes, I'm thinking about leaving the old code behind in the abstract parent
class to leave the choice of which methods to override to the backends
itself.
i would probably also drop the oldKDM and oldGDM paths.
If anyone isn't opposed because backwards compatibility, I can only agree.
i assume you'll implement this in a feature branch to make discussing
during devel easy?
Of course.
