[Kde-pim] RR: Add additional parameter to EntityTreeModel::entityData()

[David Jarvie]

On Thu, December 17, 2009 10:53 am, Stephen Kelly wrote:
> David Jarvie wrote:
>
>> On Wednesday 16 Dec 2009 19:54:18 Tobias Koenig wrote:
>>> On Wed, Dec 16, 2009 at 07:28:05PM -0000, David Jarvie wrote:
>>> Hej,
>>>
>>> > In KAlarm, when it overrides EntityTreeModel::entityData(const Item&,
>>> > ...), it needs to know which collection the Item belongs to.
>>> What's wrong with item.parentCollection()?
>>
>> I hadn't spotted that method. But what if an item is in virtual
>> collections as well - I need to know which collection is its parent in
>> the model.
>>
>
> the entityData methods are just for convenience. As you have a special use
> case, I recommend that you override ::data instead.

Looking at the entitytreemodel.cpp code, I think you're right - in fact, I
need to return a value for Qt::BackgroundRole also, which currently is
handled by data() and not passed through to entityData() to handle.

I think this points to a lack of API documentation for EntityTreeModel -
there's nothing to describe the difference between data() and
entityData().

Something else missing from the apidox class description is how the model
is organised - I presume that it's in a tree structure which mirrors the
Collection/Item tree structure, but it's not actually stated.

> You can then get the Item and ParentCollection from the index parameter
> passed into that. The ParentCollectionRole should give you the parent
> Collection whether it's virtual or not.

I hadn't noticed the ParentCollectionRole either. :(

I'll discard my Review Request.

Cheers,
David.

-- 
David Jarvie.
KDE developer.
KAlarm author - http://www.astrojar.org.uk/kalarm

_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/