[PATCH 6/6] Remove unnecessary temp variable

Brad Sowden code at sowden.org
Tue Jun 19 00:09:46 PDT 2012


Hi Stephan,

>>> diff --git a/sw/source/ui/docvw/edtwin.cxx
>>> b/sw/source/ui/docvw/edtwin.cxx
>>> index 04e6e73..9f6d2f2 100644
>>> --- a/sw/source/ui/docvw/edtwin.cxx
>>> +++ b/sw/source/ui/docvw/edtwin.cxx
>>> @@ -4992,12 +4992,9 @@ void SwEditWin::Command( const CommandEvent&
>>> rCEvt )
>>> rSh.IsCrsrReadonly();
>>> if(!bIsDocReadOnly)
>>> {
>>> - QuickHelpData aTmpQHD;
>>> if( pQuickHlpData->m_bIsDisplayed )
>>> - {
>>> - aTmpQHD.Move( *pQuickHlpData );
>>> pQuickHlpData->Stop( rSh );
>>> - }
>>> +
>>> String sWord;
>>> if( rSh.HasDrawView() && rSh.GetDrawView()->IsTextEdit() )
>>> {
>>
> ...and looking closer, are you sure that aTmpQHD is really unnecessary
> anyway? Note that QuickHelpData::Move modifies its argument (which it
> takes by non-const reference)---what it effectively does is clear
> pQuickHlpData->m_aHelpStrings, and I'm not sure whether that is relevant
> or just some misguided optimization.


I believe it's unnecessary.  In the pQuickHlpData->Stop(rSh) call that 
immediately follows Move() ClearCntnt() is called which then clears 
pQuickHlpData->m_aHelpStrings i.e. same result at the eliminated Move() 
call.

void QuickHelpData::Stop( SwWrtShell& rSh )
{
     if( !m_bIsTip )
         rSh.DeleteExtTextInput( 0, sal_False );
     else if( nTipId )
         Help::HideTip( nTipId );
     ClearCntnt();   <-------------------
}

void QuickHelpData::ClearCntnt()
{
     nLen = nCurArrPos = 0;
     m_bIsDisplayed = m_bAppendSpace = false;
     nTipId = 0;
     m_aHelpStrings.clear();   <---------------
     m_bIsTip = true;
     m_bIsAutoText = true;
}

BTW, the following comment is in my original commit message as I did 
think about the point you raised. Hopefully it's right :-)

"The only impact aTmpQHD.Move() has on pQuickHlpData is clearing
m_aHelpStrings but this is done anyway in the pQuickHlpData->Stop()"

Cheers,
Brad
P.S. point noted about keeping braces even if the IF statement only has 
a single statement.


More information about the LibreOffice mailing list