2015-05-17 10:58 GMT+02:00 Jürgen Spitzmüller <sp...@lyx.org>:
> 2015-05-17 10:57 GMT+02:00 Jürgen Spitzmüller <sp...@lyx.org>:
>
>>
>> OK, found it. Since the population of the frameColorCO depends on the
>> population of backgroundColorCO (the existence of the "none" entry), it has
>> to be populated after that one.
>>
>
BTW this demonstrates the difference between the two approaches. Why did
your method work and not insert an extra "none" item, while mine did? Here
is the explanation:
I have checked:
if (backgroundColorCO->findData("none") == -1)
so I have really checked if the backgroundColorCO has a "none" entry. And
it hadn't, because the combo was not filled yet when the signal was emitted
for the first time! So the text rightly returned "true" and the none entry
was added.
It was the subsequent filling method which then added the second one, and
this is the actual bug here! (solved by swapping the fillings of the two
combos, but additionally, I think the fill methods should first clear the
combos).
You have checked:
if (backgroundColorCO->count() == color_codes_.count() - 1)
This returned "false" and no entry "none" was added. But why did it return
false? Not because backgroundColorCO->count() == color_codes_.count(), as
you assume here, but because backgroundColorCO->count() == 0!
So this only worked by chance and actually covered the real bug.
Would you have checked
if (backgroundColorCO->count() != color_codes_.count())
you would have faced the same problem.
So, ironically, your test worked in this case just because it does not
actually test what it is supposed to test.
In other words, testing for count values means just guessing whether a
given entry is in a combo, whereas using findData really looks up the item
in the combo. This is one reason why view/data separation should always be
used.
Jürgen
