Re: [Libreoffice] Help fixing ImplHandleInputLangChange implementation

[Jesús Corrius]

>        It can help debugging if methods always return from the same place; but
> then it can help code readability if it is not incredibly deeply
> indented.

I am from the "code readability" school of programing :)

>
>> Thanks for the help! :)
>
>        Sorry for not helping :-) just do what you think fits best with that
> module's practise and style.

The problem is that there's no consistency at all in that module and
it's done both ways without much sense :)
I guess the Windows vcl code is a good candidate for a good code review...

-- 
Jesús Corrius 
Document Foundation founding member
Mobile: +34 661 11 38 26
Skype: jcorrius | Twitter: @jcorrius
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice

Re: [Libreoffice] Help fixing ImplHandleInputLangChange implementation

[Michael Meeks]

Hi Jesus,

On Mon, 2011-06-20 at 20:47 +0200, Jesús Corrius wrote:
> What's the preferred way to fix it?

:-)

> a) put everything in an if (pFrame) {} block
> b) if(!pFrame) return;

It can help debugging if methods always return from the same place; but
then it can help code readability if it is not incredibly deeply
indented.

> Thanks for the help! :)

Sorry for not helping :-) just do what you think fits best with that
module's practise and style.

Thanks !

Michael.;

-- 
 michael.me...@novell.com  <><, Pseudo Engineer, itinerant idiot


___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice

[Libreoffice] Help fixing ImplHandleInputLangChange implementation

[Jesús Corrius]

Hi all,

I'd like to fix this function implementation, as it makes my eyes cry ;)

Notice that if pFrame == NULL, the function will crash anyway because
it's only checked in the first if block and later is dereferenced.

static void ImplHandleInputLangChange( HWND hWnd, WPARAM, LPARAM lParam )
{
ImplSalYieldMutexAcquireWithWait();

// Feststellen, ob wir IME unterstuetzen
WinSalFrame* pFrame = GetWindowPtr( hWnd );
if ( pFrame && pFrame->mbIME && pFrame->mhDefIMEContext )
{
HKL hKL = (HKL)lParam;
UINTnImeProps = ImmGetProperty( hKL, IGP_PROPERTY );

pFrame->mbSpezIME = (nImeProps & IME_PROP_SPECIAL_UI) != 0;
pFrame->mbAtCursorIME = (nImeProps & IME_PROP_AT_CARET) != 0;
pFrame->mbHandleIME = !pFrame->mbSpezIME;
}

// trigger input language and codepage update
UINT nLang = pFrame->mnInputLang;
ImplUpdateInputLang( pFrame );

// notify change
if( nLang != pFrame->mnInputLang )
pFrame->CallCallback( SALEVENT_INPUTLANGUAGECHANGE, 0 );

ImplSalYieldMutexRelease();
}

What's the preferred way to fix it?

a) put everything in an if (pFrame) {} block
b) if(!pFrame) return;

Thanks for the help! :)

-- 
Jesús Corrius 
Document Foundation founding member
Mobile: +34 661 11 38 26
Skype: jcorrius | Twitter: @jcorrius
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice