ceylon / ceylon-js

DEPRECATED
Apache License 2.0
54 stars 9 forks source link

Comparing String with Integer in dynamic block gives inconclusive results #604

Closed dhoehmann closed 9 years ago

dhoehmann commented 9 years ago

Example:

dynamic {
        dynamic a = "3";
        dynamic b = 20;
        print(b<=>a);
        print(a<=>b);
    }

yields larger, larger a = "01", b = 1 yields equal, smaller a = "a", b = 1 yields larger, larger a = "10", b = 2 yields smaller, smaller

Is this JS behaviour?

quintesse commented 9 years ago

You have to think this is still Ceylon code in some way, even if its "dynamic", so what happens is that in the first case you're comparing an Integer of value 20 to a something that is not an Integer but it will be converted to that on-the-fly, so the comparison is numerical and 20 is indeed larger than 3.

In the second case you're comparing a String of value 3 to something that's not a String but it will be converted to it, so the comparison is basically textual and in that case "3" is indeed larger than "20".

So I don't think this is a bug @gavinking , unless you have an idea how this could be done differently.

gavinking commented 9 years ago

Well the question is whether we should try to protect the user's dynamic code from JavaScript's bullshit implicit type conversions. And whether it's reasonably possible to do that, or whether we just have to live with them. WDYT, @chochos?

quintesse commented 9 years ago

I always thought that using dynamic came with the implicit warning of "There be dragons here!" plus a healthy dose of "Trust me, I know what I'm doing".

gavinking commented 9 years ago

Well sure it does. But that doesn't mean we can't try to minimize the dragons.

dhoehmann commented 9 years ago

If this is the right snippet from the language module then it's a bug:

JSNum$proto.compare = function(other) {
var value = this.valueOf();
return value==other ? getEqual() : (value&lt;other ? getSmaller():getLarger());
}

this should read something like:

JSNum$proto.compare = function(other) {
var value = this.valueOf();
return value==other ? getEqual() : (value&lt;other ? getSmaller() : (value&gt;other ? getLarger() : throwIncomparableException)));
}

Converting "10" to Int when comparing is ok to me, but "a" > 1 gives false in JS, so "a" <=> 1 should not result in larger in Ceylon (and neither smaller because "a" < 1 also evaluates to false)

quintesse commented 9 years ago

@dhoehmann we're not dealing with JavaScript's rules here but Ceylon's. And doing "a" <=> 1 is basically the same as saying "a".compare(1.string) which correctly returns larger.

dhoehmann commented 9 years ago

But why does 1 <=> "a" also return larger?

quintesse commented 9 years ago

Well you can see that in the code you pasted above, it's not equal and it's also not smaller (in JS 1 < "a" returns false) so it returns larger. It's a nonsense result of course, but that's what happens when you try to compare two different types within a dynamic block that doesn't warn you that you're trying to do something that doesn't make sense.

chochos commented 9 years ago

Well IIRC we said that using dynamic meant all bets are off. To answer the original question: Yes, that's JS behavior. We could throw in both compare implementations, but I don't think that's very JS-y

dhoehmann commented 9 years ago

It's a nonsense result of course and exactly that is the point In JS comparing 1 with "a" always returns false and that's conclusive, because 1 is neither larger nor smaller nor equals "a". The <=> operator could not reflect this behaviour, so it should raise an error instead of returning larger (when JS clearly states it is not)

chochos commented 9 years ago

So now comparing between integer and string will throw a TypeError. Also, ops such as +, -, *, / and % will try to call the corresponding method if available, otherwise the native JS op will be used.