[PATCH] dix: add valuator_mask_fetch_double()

[Daniel Stone]

Hi,

On 1 October 2011 00:32, Chase Douglas <chase.douglas at canonical.com> wrote:
> On 09/30/2011 04:07 PM, Daniel Stone wrote:
>> e.g.:
>> double valuator_mask_fetch_double(struct valuator_mask *mask, int index)
>> {
>>     if (valuator_mask_isset(mask, index)
>>         return valuator_mask_get_double(mask, index);
>>     else
>>         return 0.0;
>> }
>
> Two issues:
>
> * Can we be sure that 0.0 will never be a valid value in the mask? My
> hunch is no.

No, but people who need to differentiate between 0.0 and NaN can use
valuator_mask_isset, just as before.

> * If we did this, we should do the same for normal int values. I can
> guarantee that 0 is a valid value in some cases, which makes this
> impossible. Ergo, we shouldn't do this for doubles either.
>
> If you really want to clean things up more, you could do:
>
> double valuator_mask_choose_double(struct valuator_mask *mask, int
>                                   index, double value_if_unset)
> {
>    if (valuator_mask_isset(mask, index)
>        return valuator_mask_get_double(mask, index);
>    else
>        return value_if_unset;
> }
>
> (I don't really like the name, but couldn't come up with a better one in
> 2 minutes.)

Whichever value you pick will still be valid as a ValuatorMask value.

> This would condense code blocks from:
>
> if (!valuator_mask_fetch_double(mask, i, &value))
>    value = prev_value;
>
> to:
>
> value = valuator_mask_choose_double(mask, i, prev_value);

Still though, it's just changing it from:
if (valuator_mask_isset(&mask, i))
    value = valuator_mask_get_double(&mask, i);
else
    do_something_else();
to:
value = valuator_mask_fetch_double(&mask, i, SENTINEL_CONSTANT);
if (value == SENTINEL_CONSTANT)
    do_something_else();

Which doesn't seem massively valuable to me.

> It would also satisfy the ability to use it in places where you don't
> really need a variable:
>
> if (valuator_mask_choose_double(mask, i, prev_value) > 0) { ... }

This could just be:
if (valuator_mask_get_double(&mask, i) != prev_value) { ... }
?

Cheers,
Daniel