Re: SfxItemSet::HasItem method

On Tue, 2012-02-07 at 13:56 +0100, Lubos Lunak wrote:
> On Monday 06 of February 2012, Kohei Yoshida wrote:
> > Hi there,
> >
> > I've just added a new method to SfxItemSet to provide an easy way to
> > check whether or not an item is already set, and if it's already set,
> > get that item.  Here is what I've added:
> >
> > bool SfxItemSet::HasItem(sal_uInt16 nWhich, const SfxPoolItem*& rpItem)
> > const {
> >     return SFX_ITEM_SET == GetItemState(nWhich, true, &rpItem);
> > }
>
>  I would suggest to make the second argument 'const SfxPoolItem **ppItem = 0', 
> not only to keep it consistent with GetItemState(), but also to make it more 
> visible that it is an out argument. The intuitive reading of "if( 
> set.HasItem( which, item ))" to me is "does 'set' have an item of 
> type 'which' that is 'item'?".

Yeah.  I'd thought about it, but I'd decided to favor *& over ** purely
for my personal preference.

But then now that you mention it, maybe you have a point, and I agree
that having an extra & may make it more obvious that it's an optional
argument.

I'll make that change.

Kohei

-- 
Kohei Yoshida, LibreOffice hacker, Calc