applet browser: code complexity in the data model

[Aaron J. Seigo]

hi ..

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), 
&(extraPluginAttrs[info.pluginName()])));

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 
of KCategorizedItemsViewModels::DefaultItemModel.

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:

Plasma::Applet::listAppletInfo(QString(), m_application)

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

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
Type: application/pgp-signature
Size: 197 bytes
Desc: This is a digitally signed message part.
Url : http://mail.kde.org/pipermail/plasma-devel/attachments/20090628/59c21ef4/attachment.sig