Re: [Libreoffice] [PATCH] Replace ENSURE_OR_* macros with regular code.

Hi,

I am somehow not yet convinced. I would want to wait for a second opinion.

Thanks!
Muthu Subramanian

On 01/23/2012 03:38 PM, Marcel Metz wrote:
> Hello Muthu,
>
> On 23.01.2012 09:19, Muthu Subramanian K wrote:
> > Before pushing, it would be great to understand the reason behind
> > replacing the macro with regular code.
>
> Sure thing. The reason why I replaced the ENSURE_OR_* macros with
> regular code was the replacement of the OSL_ENSURE macro with SAL_WARN.
> SAL_WARN needs a location parameter that could be added to the macros
> parameter list. But the macros lump together debugging code and code
> that is independent of the debug compilation and part of the regular
> application execution (even if in some places this is only avoiding some
> invalid application state).
> I don't have enough understanding of the code yet to think of a better
> replacement of macros where this would be applicable (exceptions,
> rewrite the code in a way that the condition can't occur anymore) so
> I've decided for now to drop the macros in favor of regular code.
>
> > I believe the macro would help in improving code readability (and
> > of course the code size) - in my understanding. Any particular
> > reason, please?
>
> I oppose to the "macros improve readability" argument, these macros
> don't improve readability, they hide some regular execution from the
> programmer. You would need to know the structure of the macro to
> completely understand what the code is doing which could be a problem
> for developers that are not aware of the side effects of the macros.
> When reading the regular code the keywords that change the execution
> would stand out while the macros could easily be skipped.
>
> The code size shouldn't matter because after the preprocessor run the
> output is exactly the same and I prefer code clarity over source code size.
>
> regards Marcel Metz
>
>
> _______________________________________________
> LibreOffice mailing list
> LibreOffice@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/libreoffice