Why is the mark podcasts as seen option gone?
Leo Franchi
lfranchi at kde.org
Sun Mar 28 19:22:40 CEST 2010
On Sun, Mar 28, 2010 at 1:00 PM, Mathias Panzenböck
<grosser.meister.morti at gmx.net> wrote:
> On 03/28/2010 05:40 PM, Mathias Panzenböck wrote:
>> Also the "unsubscribe" action for podacast channels is pretty broken:
>> * Sometimes the wrong channel is asked to be removed.
>> * The channel to be removed seems to be associated multiple times with the action and so you have
>> to confirm/cancel several times!
>
> In PodcastView::actionsFor the indexes are the right ones. Somewhere in the awfully hacky way the
> actions are gathered and filled with the channels/episodes the error happens. Why do we have to do
> that in this very hacky way? Why not do something like this in actionsFor (using Python as pseudocode):
>
> PodcastView:
> def sectionsFor(self,indexes):
> actionMap = {}
> orderedActions = []
> for index in indexes:
> item = index.data( DataRole )
> for action in items.actions():
> try:
> items = actionMap[action]
> except KeyError:
> items = []
> actionMap[action] = items
> orderedActions.append(action)
> items.append(item)
>
> for action, items in actionMap.items():
> action.setData(items)
>
> return orderedActions
>
> Then we can remove e.g. the parameter to SqlPodcastProvider::channelActions and skip messing with
> the action date there altogether. I think it would be more clean if one could pass an argument to
> the action callback in some way other than by setting the data member of the action. I think setting
> this member is extremely ugly and error prone because it is a semi-global state, a unexpected side
> effect. If it would be possible to pass a argument to the action slot one could remove the line
> "orderedActions.append(action)" and instead of the line "action.setData(items)" write:
> orderedActions.append(WrapperAction(action,items))
>
> The WrapperAction gets deleted after it is used. Well I guess in C++ this is more complicated.
>
> So basically I'm asking why do we need to forbid access to the internalPointer() of the
> PodcastModel? So many things would be so much more easy and, as this example shows, much less error
> prone if one could just access this pointer.
Side note: never access a model's internalPointer() from anything
outside the model itself. You cannot be sure of what it is. If in the
future the underlying implementation of the model changes, this will
break. If in the future there is a proxy model added in between, this
will break. Casting from internalPointer is *only* to be done in the
model itself. It is a public method due to C++ quirks, not because it
is meant to be access externally. Use data() with a custom role for
all fetching calls.
leo
--
_____________________________________________________________________
leonardo.franchi at tufts.edu Tufts University 2010
leo at kdab.com KDAB (USA), LLC
lfranchi at kde.org The KDE Project
More information about the Amarok-devel
mailing list