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

[Muthu Subramanian K]

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 at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/libreoffice
>