[FFmpeg-devel] [PATCH] OpenEXR decoder rev-12

[Reimar Döffinger]

On Wed, Jul 29, 2009 at 03:09:55PM +0200, Jimmy Christensen wrote:
> On 2009-07-29 15:06, Reimar D?ffinger wrote:
>> On Wed, Jul 29, 2009 at 02:39:08PM +0200, Jimmy Christensen wrote:
>>>> That's what functions are there for. I think in quite a few places readability
>>>> could be improved quite a bit by creating a well named function, but I think
>>>> you so far ignored all my suggestions in that regard.
>>>
>>> It's not as much as I ignored them. I tried making them into functions
>>> but since it needs to exit with a return -1, I didn't know how to make
>>> the function make the main function return -1.
>>
>> E.g.
>> If (get_value(...)<  0)
>>      return -1;
>> That still duplicates the if, but should still be less code, at least
>> the av_log is centralized.
>> You could then consider if its reasonable to wrap even that into a macro.
>
> This was also pretty much the conclusion that I got to. For most of the  
> functions it wouldn't make the code much less.

But it will also give the code a name and a doxygen description.
Also, it will help you avoid doing it one way in five place and a different
way in a another (as you have done a few times), leaving in a few years everyone
scared to change the code because the can't know if that case was done differently
on purpose.