applet browser: code complexity in the data model
Aaron J. Seigo
aseigo at kde.org
Mon Jun 29 01:00:08 CEST 2009
so, having looked through the code there are a number of different issues i'd
like to address ... there's a lot of good ideas in there already and it's
great to see code written and working. so take this as constructive critique
hopefully building on what's already there ;)
so .. code complexity.
the code structure of the old applet dialog was highly engineered with several
layers of indirection in the code. it was not easy to get into and that,
coupled with "well, it basically works as it is now", is why it never really
got much work done on it.
we should try and avoid the same situation happening again and keep the code
as straight-forward as possible. the parts of plasma that are easiest to
approach get the most support. the parts that are less so end up getting
maintained by just one person. that's a pretty low bus-number ;)
so .. first, it's model-view based. i'm not particularly sure that's called
for in this case, but if we go this route, let's KISS it. :)
there are lines like this:
appendRow(new PlasmaAppletItem(this, attrs,
((m_favorites.contains(info.pluginName())) ? PlasmaAppletItem::Favorite :
PlasmaAppletItem::NoFilter) | ((m_used.contains(info.pluginName())) ?
PlasmaAppletItem::Used : PlasmaAppletItem::NoFilter),
oh no. :)
so ... here's what i suggest in the model:
right now PlasmaAppletItemModel is a
KCategorizedItemsViewModels::DefaultItemModel, which is just a
QStandardItemModel. that's at least one level of indirection too many. get rid
in fact, forget about PlasmaAppletItemModel being a QStandardItemModel. it's
just overhead in this case: we already have a perfect container object. so
PlasmaAppletItemModel (PAIM? :) should just subclass QAbstractItemModel.
next, get rid of PlasmaAppletItem. we don't need it.
there is this line in PAIM::populateModel:
so now we have a list of KPluginInfos. perfect! but what does the code do with
them? it extracts all the information from them and sticks them in
PlasmaAppletItems! meh. just hold onto the KPluginInfo::List.
then implement data() such that the index.row() refers the index in the plugin
info list, and the column to the various bits of data.
this brings us down from 4 classes to 1 and eliminates two layers of
this will make the implementation of the UI bits soooo much easier because you
won't have to spend a bunch of time reading the model code.
i'm quite happy and willing to work on this with you. if you tell me what your
working hours are, we can hook up on irc. the above is really 2-3 hours of
work at most, however.
Aaron J. Seigo
humru othro a kohnu se
GPG Fingerprint: 8B8B 2209 0C6F 7C47 B1EA EE75 D6B7 2EB1 A7F1 DB43
KDE core developer sponsored by Qt Software
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Size: 197 bytes
Desc: This is a digitally signed message part.
Url : http://mail.kde.org/pipermail/plasma-devel/attachments/20090628/59c21ef4/attachment.sig
More information about the Plasma-devel