[MPlayer-cvslog] r33013 - in trunk/gui: app.c app.h skin/skin.c

[Ingo Brückl]

Alexander Strasser wrote on Thu, 3 Mar 2011 20:09:22 +0100:

>> Author: ib
>> Date: Thu Mar  3 14:13:20 2011
>> New Revision: 33013
>>
>> Log:
>> Remove parameter from appInitStruct() function.
>>
>> Since there is only one listItems structure variable (the global appMPlayer),
>> there is no need to pass it to the function.

> I don't think this change is desirable.
> IOW I leave further decision to you.

The main reason I did this was because I wanted to add freeing the fonts
there.

Actually, the fonts would belong into the listItems structure as all the
other skin items, too, but unfortunately their storage is currently realized
differently. If the function would have kept its parameter, the whole would
have looked that way:

void appInitStruct(listItems *item)
{
    frobnicate(&item->member);
    ...
    fntFreeFont();
}

i.e. it would have been possible that there are different listItems but only
one font array. As you might already see looking at the code (fntFreeFont()
independent from item) this is wrong and could lead to mistakes because the
fonts always belong to the skin items which means that there cannot be two
listItems with only one font array.

So I had two alternatives. Either the fonts become part of the listItems
structure, what would have required a lot of changes and only for the one
purpose to have it in the only (global) variable appMplayer (i.e. all calls
would only have been done with appMplayer anyway), or else make a independent
treatment of listItems and fonts much less likely by design. I have decided
in favour of the second, simpler way.

> code is marginally simplified on two lines,

It was not about simplification of the code itself.

> the init function now is less flexible, reusable and testable

It has only the one purpose of dealing with appMplayer (and this is exactly
what it does now).

> (because it now explicitly depends on global state)

That was the goal.

> future modifications might get harder/bigger (like allocating the structure
> e.g. on the heap or renaming the global)

If this would become necessary, there would be a lot of other problems,
because the global appMPlayer is *everwhere*.

It may seem that the init function has lost usefulness, but actually there
are no other listItems (the listItems structure *is* the skin resp. the GUI
application).

I hope this clarifies why I did it.

Ingo