[Development] Should QVariant be doing fuzzy comparisons on doubles?

[Edward Welbourne]

On Fri, Sep 23, 2016 at 09:56:30AM +0000, Edward Welbourne wrote:
>> Indeed; having all values of some type equivalent is worse (for the "has
>> my value changed" test you illustrate, at least) than having a value not
>> equal to itself (which triggers a "changed" event for a non-change).  In
>> fact one can have one's mathematical purity by attacking the "for all x"
>> part of the above instead of the "x == x" part.  Although it's usual to
>> talk about "a relation is reflexive on S if, for all s in S, it relates
>> s to s"

André Pönitz replied:
> Right. That's pretty much the definition the known universe uses:
> https://en.wikipedia.org/wiki/Reflexive_relation
> (and no, *I* didn't put it there)

Like I say, that's the usual definition.  The interesting thing that
tends to get lost as a result is that the only part it actually adds to
the definition of an equivalence is the "for all s in S" part since,
when a symmetric transitive relation relates x to y it also relates (by
symmetry) y to x and thus (by transitivity) x via y to x and y via x to
y.  So a symmetric transitive relation is already reflexive *on its set
of values* - and the usual definition of equivalence only *needs* the
usual definition of reflexive as a way to say *and every member of S is
a value of the relation*; the "relates each to self" part is already
taken care of.  Hence my attack on the "for all x" side.

>> one can equally define reflexive by "whenever the relation
>> relates any x to y, it also relates x to x and y to y".
>> (This is the
>> more natural formulation in a mathematics based on relations [*] rather
>> than sets.)
>>
>> [*] http://www.chaos.org.uk/~eddy/maths/found/relate.xhtml#Types
>>
>> With that formulation (which doesn't tie the relation to a set that it's
>> "on"), returning false for the types that don't support equality
>> comparison works; we won't consider any value of such a type equal to
>> *anything*, so our relation isn't required to relate them to themselves.
>> We are then left with QVariant::operator== being a relation formally on
>> only those QVariants that support comparison, not on all QVariants
>> (although it tolerates being asked about the values it doesn't relate to
>> anything; it just says no about them).

> How does that help with QHash<QVariant, ...> which needs comparsion of the
> keys?

It makes clear what the problem is - you can't sensibly use a value as a
key in a hash if it doesn't compare equal to itself.  Your problem isn't
that you need x == x, it's that you need it for all QVariants and it
isn't a sensible thing to ask for among all QVariants, because some of
them don't know *how to ask whether* x == x.

You can't sensibly do comparison on the values of types for which no
operator== is registered, so you can't sensibly make a hash that has
them as keys.  So if you want QHash<QVariant,...> you have to either
live with the same kind of brokenness that several contributors to this
discussion (from both camps) have described for equality or you have to
live with your hash refusing to tolerate keys of types (within QVariant)
that don't support equality comparison.

After all, when you come to look up an entry in your hash, you do need
to do a comparison (once you've found the right hash bucket to look in),
to check you haven't just got a hash collision - right ?  So if you
can't do comparisons sensibly with values of some type (that you've
wrapped in QVariant), then you can't sensibly use those values as keys
in a hash.

>> It's not ideal (and the justification is heterodox) but false is a valid
>> answer for the incomparable values,

> This is a nonsensical line of argument long as you try to use it to counter
> "unexpected" results.

I'm not trying to use it to counter unexpected results.  I'm trying to
tell you that you should expect perverse results and not implement APIs
whose sanity depends on sensible behaviour from a "solution" to an
ill-posed problem.

> With your "uncomparable things compare false" approach QHash<>::contains()
> will never return true for uncomparable QVariants, i.e. code like
>
>     void doSomethingOnlyOncePerValue(QVariant v)
>     {
>         static QHash<QVariant, int> seen;
>         if (seen.contains(v))
>             return;
>         seen.insert(v, 0);

For reference, I think QHash::insert(K key, ...) should start by
checking key == key; if that fails, refuse to insert.

>         doSomething(v);
>     }
>
> will be quite surprising, too.

*Any* resolution of the problem will present surprises, if we allow
QHash<QVariant,...> to use keys for which we don't know how to do
comparison.  If QHash has no way to reject such keys, then supporting
QVariant as a key type is *inescapably* problematic.  A hash needs a
comparison relation that it can use on its keys that is an equivalence
and for which each value offered as a key is equal to itself; so either
you need your QHash<QVariant,...> to actually have a key-type that's a
proper sub-type of QVariant ("return false;", excluding sub-types for
which we have no registered operator==) or you have to treat all values
of each incomparable sub-type as if they were equal ("return true;").
Either shall have its defects and produce perverse behaviour.

When you are faced with evidence the hole you're digging is just going
to engulf you, it tends to be a smart move to stop and ask why you're
still digging the hole and whether there's something better you could be
doing instead.

	Eddy.