Proposal-temporal: Comparison functions

Created on 10 Jul 2017  路  33Comments  路  Source: tc39/proposal-temporal

Should comparison be part of the proposal? I realize it's more awkward without operator overloading. And for instants it's easy to convert to numbers and compare. But it seems important to be able to compare PlainDate/PlainTime/PlainDateTime. (Is this day equal to X's birthday? Is the time between 9am and 5pm?)

I think it'd be good if comparison between unlike types threw, at least in the beginning, instead of trying to do some clever conversion.

behavior

Most helpful comment

At this point, it doesn't really matter what's most efficient. What matters is that Array.prototype.sort calls .toString() on what is passed to it, and changing that is out of scope for this proposal because it would involve changing other parts of the spec than date.

All 33 comments

I think it'd be good if comparison between unlike types threw, at least in the beginning, instead of trying to do some clever conversion.

+1 if comparisons are supported

Yes. I think we should include these. :)

In-built comparison functions would also be very handy when sorting a list of PlainDates.

I would tend to agree with Dominic that some basic comparisons on like types make sense for the initial proposal. Let's make cross-type comparisons something for later or never.

/cc @littledan as I believe he had ideas on how to add named comparison functions now which can in the far future maybe become the basis for overloaded operators.

I do want to note that I'm resisting having this proposal ride on a bunch of other proposals. Both BigInt and operator overloading have come up in this context, and this proposal will be hard enough to get through as it is given the size.

For sorting, I think as long as we define the result of valueOf (#25) that should work ok.

Array.prototype.sort uses toString, not valueOf.

Oh really! That's good to know. Thanks. Another good point for #25.

How the heck does that work?

new Date().toString()
"Tue Jul 11 2017 11:04:10 GMT-0700 (Pacific Daylight Time)"

That... won't work right. Does it specifically use .toISOString() for dates?

No, you just can't use Array.prototype.sort()'s default comparator on dates effectively.

Apparently I've been using moment too long for sorts and I forgot how dates work :-)

Maybe make an exception (and a correction) for that new date system and make .sort() use .valueOf() ?

Well, that spirals into writing special cases into array.prototype.sort. One could, but I don't know how well received it would be.

That could be a "Symbol.sort" function key used when present instead of .toString() ?

If the output of PlainTime#toString is an ISO8601 string, it will sort as expected.

["01:00:00", "00:00:00", "00:00:01", "00:01:00"].sort()
["00:00:00", "00:00:01", "00:01:00", "01:00:00"]

If the output of PlainDate#toString is an ISO8601 string, it will sort as expected for 4 digit years, though 5+ digit years would not sort correctly unless left zero filled.

Would not it be more efficient to sort by a floating point or decimal value instead of converting to strings and then sort by comparing strings ?

At this point, it doesn't really matter what's most efficient. What matters is that Array.prototype.sort calls .toString() on what is passed to it, and changing that is out of scope for this proposal because it would involve changing other parts of the spec than date.

Wait, wait, there's a mistake here.

Array.prototype.sort()'s default implementation should call Symbol.toPrimitive (spec) on any given values that have it:

a = {name: 'a', [Symbol.toPrimitive]:(hint) => console.log('1', hint) || 2}
b = {name: 'b', [Symbol.toPrimitive]:(hint) => console.log('2', hint) || 1}
[b, a].sort().map(o => o.name)
> 1 string
> 2 string
> ["b", "a"]

This is _different_ to calling .toString(). In theory one _could_ implement a [Symbol.toPrimitive] function that just returns timestamps. Quoth the spec (emphasis mine):

If an object is capable of converting to more than one primitive type, _it may use the optional hint PreferredType to favour that type_. Conversion occurs according to Table 9:

Yes, sorry, I meant to capitalize ToString, not toString.

More problematic, as ToString _will_ force any ToPrimitive into a String type. Looks like operator overloading is needed!

Well, that'll teach me for not reading the spec! I had no idea Array.prototype.sort used toString().

No, you just can't use Array.prototype.sort()'s default comparator on dates effectively.

Wow, yeah, I guess I'd only been sorting explicit ISO string representations previously, because I hadn't run into that behaviour before. But now that I test it out...

[1, 2, 3].map(n => new Date(2017, 6, n)).sort();
/*
[
  Mon Jul 03 2017 00:00:00 GMT+1000 (AEST),
  Sat Jul 01 2017 00:00:00 GMT+1000 (AEST),
  Sun Jul 02 2017 00:00:00 GMT+1000 (AEST)
]
*/

About operator overloading: I was thinking that maybe we could start with static methods on constructors, and then upgrade to operator overloading. However, in the design of BigInt, that approach was rejected by people I talked with on the committee in favor of just having the operators overloaded.

The default sort comparison is entirely busted. I don't see a point in trying to overload new dates to make it fixed. As long as there's an effective comparison method that can be called directly, or passed in as an argument, it seems good to me.

If we want to make comparison to other types with < throw, how about making the valueOf method exist and throw? We could patch SortCompare, but really, given how entirely broken it would remain, that seems like "putting lipstick on a pig"; we obviously can't make .toString() throw in general.

I don鈥檛 believe this is something that鈥檚 being done in the scope of this project, and therefore am closing it. Please feel free to reopen. Adding the deferred label so we can probably come back to this later.

I don't have privileges to reopen, but I think this should be in the scope of the project.

datetime-comparison would be trivial (w/o overloading) if javascript sticks with [utc] ISOStrings for most of its business-logic -- and this proposal instead focused on static-functions that manipulated [utc] ISOStrings instead of classes.

@mj1856 @pipobscure @sffc what do you all think about this?

I think this is within the scope of the proposal if someone can champion this bug and figure out the right way to specify the comparison function.

I believe the current proposal is to add such a comparison function.

Correct, and in fact more than one.

Can we close this issue, then?

We still need spec text.

Closing, because we have a decision and we're trying to find what needs to be discussed.

Was this page helpful?
0 / 5 - 0 ratings