Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow tuples to be compared with order operators (<, >, <=, >=) #216

Closed
2 tasks done
Jamesernator opened this issue Jan 24, 2021 · 22 comments
Closed
2 tasks done

Allow tuples to be compared with order operators (<, >, <=, >=) #216

Jamesernator opened this issue Jan 24, 2021 · 22 comments

Comments

@Jamesernator
Copy link

  • My issue is not a duplicate
  • My issue relates to the Records and Tuples proposal and not a follow-on proposal

I would like to propose allowing for comparing tuples lexographically (e.g. similar to Python). This would allow simpler implementations for sorting by multiple keys.

This is useful both when sorting arrays, as well as for other applications that need to compare things by multiple keys.

For example a priority queue could use this to sort by an arbitrary priority rule of it's entries:

class PriorityQueue {
  #heap = [];
  #toPriority;
  constructor(toPriority=value => value) {
    this.#toPriority = toPriority;
  }

  enqueue(value) {
    const priority = toPriority(value);
    const index = this.#heap.length;
    this.#heap.push(value);
    while (index > 0) {
      const parent = Math.floor(index / 2);
      if (toPriority(parent) >= priority) break; // It's bubbled up to correct position
      [this.#heap[parent], this.#heap[index]] = [this.#heap[index], this.#heap[parent]];
      index = parent;
    }
  }
  
  dequeue() {
    // Similar with priority comparisons in dequeue
  }
}

// NOTE: This is overly simplified, a proper implementation would also allow for updating
// priority after the fact using IntersectionObserver or similar, but that doesn't change
// the usage of toPriority
const elementsToLoad = new PriorityQueue(image => {
  // Load images connected to the document first, for images in the document we load them
  // in order based on their distance to the viewport
  return #[image.isConnected ? 0 : 1, distanceToViewport(image)];
});
@ljharb
Copy link
Member

ljharb commented Jan 24, 2021

What would it mean to sort Symbols?

The concept of generically sorting values, and thus also sorting containers of values, was rejected by the committee in June as part of the https://github.com/hemanth/generic-comparison proposal.

@bakkot
Copy link
Contributor

bakkot commented Jan 24, 2021

What would it mean to sort Symbols?

I would assume that t < x where t is a tuple containing a symbol would throw, just as comparing Symbol() < x does.

@ljharb
Copy link
Member

ljharb commented Jan 24, 2021

That’s certainly a choice that can be made. How would you sort a Box / object?

@bakkot
Copy link
Contributor

bakkot commented Jan 24, 2021

I would assume "sorting lexicographically" means that #[x] < #[y] would be equivalent to x < y for any x and y, so, it would call valueOf on the object and compare using the resulting primitive.

@ljharb
Copy link
Member

ljharb commented Jan 24, 2021

I’m skeptical about the utility of this with those behaviors, but if the champions want to adopt it, I’ll look forward to the presentation that makes the case.

@bakkot
Copy link
Contributor

bakkot commented Jan 24, 2021

The utility is presumably in contexts where you have lists of numbers or strings.

@ljharb
Copy link
Member

ljharb commented Jan 24, 2021

Sure, but that was the case for the comparison proposal as well, for arrays of numbers or strings - and the committee rejected it, so I’d expect either both or neither to be acceptable.

@bakkot
Copy link
Contributor

bakkot commented Jan 24, 2021

and the committee rejected it

That was not my impression from the discussion, which focused very heavily on generic comparison and the spaceship operator and did not, to my recollection, raise the possibility of adding a method which specifically did lexicographic comparison of arrays (i.e. one which deferred directly to < on the underlying values). I suspect the committee would have been much more receptive to such a proposal; at the very least, I would not take that discussion as strong evidence that it would be opposed to it.

@ljharb
Copy link
Member

ljharb commented Jan 24, 2021

Interesting to hear your different take. Lexicographic comparison of arrays was the motivating example for the proposal, certainly.

@Jamesernator
Copy link
Author

That was not my impression from the discussion, which focused very heavily on generic comparison and the spaceship operator and did not, to my recollection,

Interesting to hear your different take. Lexicographic comparison of arrays was the motivating example for the proposal, certainly.

It does seem that one of the concerns in the notes is that <=> would not necessarily match the behaviour with </>/<=/>=. For arrays this would lead to situations like [10] <=> [2] === 1 but [10] < [2] === true.

Tuples being new don't suffer the legacy < toString behaviour and so should be safer for sorting.


As an aside, the whole 3-way sort thing I've always found to be an awkward design of array .sort(), and would love to see something similar to lodash's sortBy in the language.

e.g. With tuples it could be as simple as:

// Sort by name, then by age
const sorted = people.sortBy(person => #[person.name, person.age]);

But that would be a separate proposal.

@rricard
Copy link
Member

rricard commented Feb 25, 2021

@Jamesernator can you confirm if the following example correctly represents what you are proposing here?

#[1, 1] > #[0, 1]; // => true
#[1, 1] > #[1, 0]; // => true
#[1, 1] >= #[1, 1]; // => true
#[1, 1] > #[1, 1]; // => false
#[1] >= #[1, 1]; // => false
#[1] >= #[1, 0]; // => ???
#[1, NaN] >= #[1, NaN]; // => ???

The last examples seem problematic to me and would push me towards thinking you should resolve this in userland javascript with a comparison function. At this point I think the idea is worth discussing but I fear we will add something strange to the proposal: having a systematic way to deal with edge cases (I am sure I missed a bunch of them!) would maybe make this less strange...

@Jamesernator
Copy link
Author

Jamesernator commented Feb 25, 2021

The second last one would be #[1] <= #[1, 0]; // => true, this falls out from usual lexographic orders, e.g. why "I" <= "Id". NaN's behaviour I have no suggestion for other than to just treat them like .sort() already does.

The last examples seem problematic to me and would push me towards thinking you should resolve this in userland javascript with a comparison function.

Even if this is the case, could we at least make </>/<=/>= throw rather than having footgunny toString behaviour? This would leave it open as a possible future extension if there proves to be more demand for it.

@rricard
Copy link
Member

rricard commented Feb 25, 2021

For the second to last one that makes perfect sense. For the last one, it seems like NaN >= NaN gives false but that would not make sense for records since we currently have #[NaN] === #[NaN] // => true, while #[NaN] >= #[NaN] and #[NaN] <= #[NaN] would be both false. Not sure that throwing here would be reasonable either just looking at the coherency aspect.

@Jamesernator
Copy link
Author

Jamesernator commented Apr 13, 2021

Not sure that throwing here would be reasonable either just looking at the coherency aspect.

Just to be clear, I wasn't meaning that if the proposal doesn't support lexical comparison, then it shouldn't allow comparison (except ==/===) at all.

i.e. Rather than adopting the footgun behaviour that Array has where [10] < [2], for tuples #[10] < #[2] should just throw a TypeError (which is also what Symbol does).

I get that for the most part Tuples are symmetric with Arrays by design, but there are other array footguns that haven't been ported over to Tuple. So I'm suggesting we shouldn't port the comparison footgun over and instead either just throw, or ideally support lexical comparison.

@Jamesernator
Copy link
Author

I think from reading the spec that throwing is the current behaviour, as as far as I can tell #[10] < #[2] will call ToNumeric on both arguments, which by the record tuple spec will throw a TypeError, which is the error behaviour I was suggesting.

@dmitri-gb
Copy link

The second last one would be #[1] >= #[1, 0]; // => true, this falls out from usual lexographic orders, e.g. why "I" <= "Id".

For the second to last one that makes perfect sense.

I'm not sure I understand the thinking behind this. This behavior seems rather unintuitive and goes against the common definition of "lexicographical order". Why does it need to differ from the corresponding string example?

After all, the result should stay the same when you remove the common prefix, right? So #[1] and #[1, 0] should compare the same as #[] and #[0]. And it only makes sense for the empty tuple to be considered the lowest element in the total order of tuples. So #[] < #[0] and therefore also #[1] < #[1, 0].

@Jamesernator
Copy link
Author

I'm not sure I understand the thinking behind this. This behavior seems rather unintuitive and goes against the common definition of "lexicographical order". Why does it need to differ from the corresponding string example?

I had the comparison around the wrong way, oops. I edited it to be the right way around.

@rricard
Copy link
Member

rricard commented Jul 12, 2021

Hi, we discusses with the champion gorup and will probably keep the current behavior for the time being which is throwing: https://tc39.es/proposal-record-tuple/#sec-tonumber

This gives the possibility to layer it on later in a follow-on proposal.

@littledan
Copy link
Member

Part of the design of Tuples is that code can be ported back and forth between Tuples and Arrays if it operates in the common subset (no mutation and no equality comparison). We have particular reasons for using ===, not a deepEquals function, for comparing R&T for equality, but I don't think those reasons apply to < for relational comparison. This proposal would expand the restriction to include no relational comparison. I would prefer to instead add lexicographical comparison in a way which also works on Arrays as well, e.g. a method or function, while steering clear from the parts that the committee objected to of the previous proposal (e.g. a new symbol-based protocol for a new <=> operator which can diverge from <).

@rricard
Copy link
Member

rricard commented Jul 8, 2022

In order to focus on changes that are necessary to finish this proposal, we are closing issues related to follow-on proposals. You are free to continue the discussion here and reference it in other venues.

@rricard rricard closed this as completed Jul 8, 2022
@theScottyJam
Copy link

If it's decided that tuples shouldn't be directly comparable with < and >, due to how <=/>= conceptually conflicts with ===, it would be nice if we still expose the comparable behavior somehow. For example, I would be happy if we could compare a .orderable property on tuples like this:

#[1, 3].orderable < #[2, 2].orderable

That way, we still get the benefit of being able to compare tuples, which is useful for many algorithms, without creating the conceptual conflict between <=/>= and ===.

@acutmore
Copy link
Collaborator

acutmore commented Oct 2, 2022

For example, I would be happy if we could compare a .orderable property on tuples like this:

So far the design of Tuples has kept the set of methods to be a subset of the Array methods. I think this subset property should would be good to maintain.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants