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

Comparable: add comparison operators and type specialization #1492

Closed
DartBot opened this issue Feb 2, 2012 · 15 comments
Closed

Comparable: add comparison operators and type specialization #1492

DartBot opened this issue Feb 2, 2012 · 15 comments
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. closed-not-planned Closed as we don't intend to take action on the reported issue

Comments

@DartBot
Copy link

DartBot commented Feb 2, 2012

This issue was originally filed by @seaneagan


It would be nice to be able to compare Comparables using comparison operators. It would also be nice to be able to specialize the types it can be compared to.

Together these would give us:

interface Comparable<T extends Comparable> {
  int compareTo(T other);
  bool operator < (T other);
  bool operator <= (T other);
  bool operator > (T other);
  bool operator >= (T other);
}

There should be mixin(s) (issue #33) to assist in implementing Comparable. Two options for what this mixin would look like:

  1. define everything in terms of compareTo:

  bool operator < (T other) => compareTo(other) < 0;
  bool operator <= (T other) => compareTo(other) <= 0;
  bool operator > (T other) => compareTo(other) > 0;
  bool operator >= (T other) => compareTo(other) >= 0;
  bool operator equals (T other) => compareTo(other) == 0;

  1. define everything in terms of < and equals:

  int compareTo(T other) => this < other ? -1 : this == other ? 0 : 1;
  bool operator <= (T other) => this < other || this == other;
  bool operator > (T other) => ! (this < other || this == other);
  bool operator >= (T other) => ! this < other;

Of the current interfaces in the core library which extend Comparable, only num currently has comparison operators, Duration, Date, and String do not. They could add them as follows:

interface num extends Comparable<num>
interface Duration extends Comparable<Duration>
interface Date extends Comparable<Date>
interface String extends Comparable<String>

@dgrove
Copy link
Contributor

dgrove commented Feb 3, 2012

Added Area-Library, Triaged labels.
Marked this as being blocked by #33.

@DartBot
Copy link
Author

DartBot commented Feb 5, 2012

This comment was originally written by @tomyeh


Consistency is important to avoid mistake. It is more important for a script language. All these kind of errors can't be found at the compile time (for example, if Dart VM, such as Dartium, is used).

@DartBot
Copy link
Author

DartBot commented Feb 13, 2012

This comment was originally written by @seaneagan


Also, Math.min/max could be generalized to work on all Comparables, by defining Comparable#min/max. The mixin implementations could look like:

T min(T other) => this <= other ? this : other;
T max(T other) => this >= other ? this : other;
// or
T min(T other) => compareTo(other) <= 0 ? this : other;
T max(T other) => compareTo(other) >= 0 ? this : other;

// Strings
'a'.min('b'); // 'a'
'a'.max('b'); // 'b'

// Dates
today.min(yesterday); // yesterday
today.max(yesterday); // today

// Durations
hour.min(second); // second
hour.max(second); // hour

@DartBot
Copy link
Author

DartBot commented Feb 13, 2012

This comment was originally written by @seaneagan


and of course:

// nums
2.min(3); // 2
2.max(3); // 3

@DartBot
Copy link
Author

DartBot commented Feb 17, 2012

This comment was originally written by @seaneagan


Comparable#compareTo could be removed as well, and <= could instead be used as the default for List#sort (and any where else that comparators are used) whose argument could be a Function which returns bool instead of int, which should have a slight performance advantage. The compareTo-based mixin above though might still be useful when implementing Comparable.

@DartBot
Copy link
Author

DartBot commented May 22, 2012

This comment was originally written by @seaneagan


In addition to min and max, clamp (issue #44) would make sense:

x.clamp(-1, 1);

These could alternatively all be static methods on Comparable if statics were allowed on interfaces (issue #2975 except without the restriction to go through a default class):

Comparable.min(2, 3) // 2
Comparable.max(2, 3) // 3
Comparable.clamp(1, 2, 3) // 2

In this case it would be desirable to have generic methods (issue #254) so that for example in:

var max = Comparable.max<int>(x, y);

it can be checked that x and y are ints, and known that max will be an int.

@DartBot
Copy link
Author

DartBot commented Jun 7, 2012

This comment was originally written by dha...@google.com


Having comparison operators on the interface is okay; having a mixin is great; but I'd strongly prefer it if the language had builtin support for Comparable so you don't even have to write these operators (as the D programming language allows).

@DartBot
Copy link
Author

DartBot commented Sep 5, 2012

This comment was originally written by @seaneagan


With the current mixin proposal:

http://news.dartlang.org/2012/08/darts-modest-proposal-for-mixins.html

this might look like:

class Comparable<T extends Comparable<T>> {
  int compareTo(T other);
  bool operator < (T other) => compareTo(other) < 0;
  bool operator <= (T other) => compareTo(other) <= 0;
  bool operator > (T other) => compareTo(other) > 0;
  bool operator >= (T other) => compareTo(other) >= 0;
  bool operator == (T other) => compareTo(other) == 0;
}

class Date mixin Comparable<Date> {
  int compareTo(T other) => millisecondsSinceEpoch.compareTo(other.millisecondsSinceEpoch);
  //...
}

class Duration mixin Comparable<Duration> {
  int compareTo(T other) => inMilliseconds.compareTo(other.inMilliseconds);
  //...
}

@alan-knight
Copy link
Contributor

Marked this as being blocked by #33.
Unmarked this as being blocked by #33.

@DartBot
Copy link
Author

DartBot commented Sep 11, 2012

This comment was originally written by @seaneagan


Including issue #3176 it could be:

class Comparable<T extends Comparable<T>> {
  int compareTo(T other);
  bool operator < (T other) => compareTo(other) < 0;
  bool operator <= (T other) => compareTo(other) <= 0;
  bool operator > (T other) => compareTo(other) > 0;
  bool operator >= (T other) => compareTo(other) >= 0;
  bool operator == (T other) => compareTo(other) == 0;
  T min(T other) => this <= other ? this : other; // alternatively "boundAbove", "minimize", etc.
  T max(T other) => this >= other ? this : other; // alternatively "boundBelow", "maximize", etc.
  T clamp(T min, T max) => this.max(min).min(max);
}

@DartBot
Copy link
Author

DartBot commented Oct 31, 2012

This comment was originally written by @seaneagan


The only place I don't think all the extra methods make sense is String due to lexicographic ordering not being a "quantitative" ordering, so I think separate Comparable and Quantity interfaces would make sense:

abstract class Comparable<T extends Comparable<T>> {
  int compareTo(T other);
}

abstract class Quantity<T extends Quantity<T>> extends Comparable<T> {
  bool operator < (T other) => compareTo(other) < 0;
  bool operator <= (T other) => compareTo(other) <= 0;
  bool operator > (T other) => compareTo(other) > 0;
  bool operator >= (T other) => compareTo(other) >= 0;
  bool operator == (T other) => compareTo(other) == 0;
  T min(T other) => this <= other ? this : other; // alternatively "boundAbove", "minimize", etc.
  T max(T other) => this >= other ? this : other; // alternatively "boundBelow", "maximize", etc.
  T clamp(T min, T max) => this.max(min).min(max);
}

class String implements Comparable<String> ...

class num mixin Quantity<num> ...
class Duration mixin Quantity<Duration> ...
class Date mixin Quantity<Date> ...
// etc.

@DartBot
Copy link
Author

DartBot commented Apr 17, 2013

This comment was originally written by @seaneagan


Now with M4, it looks like DateTime will never get comparison operators (since it already has isSameMomentAs, isBefore, isAfter), but it will be Comparable. So does that mean Comparable will not get comparison operators ?

@floitschG
Copy link
Contributor

There are no plans to make the Comparable interface include <, <=, >= and >. One of the most important implementors (double) doesn't have a correspondence between compareTo and operators.
Also, just because two instances can be ordered, doesn't mean that comparison operators make sense on them. In particular the fact that the comparison operators need to play nicely with "==" makes it difficult/impossible to implement. A compareTo function might only take one field into account, but the "==" uses many more fields.
DateTime is an example.

However, it is conceivable to create mixins that adds these methods (or inversely create a compareTo from the operators).

Example:
class OperatorComparableMixin {
  int compareTo(other) => this < other ? -1 : (other < this ? 1 : 0);
  operator <(other);
  operator <=(other) => !(other < this);
  operator >(other) => other < this;
  operator >=(other) => !(this < other)
}


Added NotPlanned label.

@DartBot
Copy link
Author

DartBot commented Apr 17, 2013

This comment was originally written by @seaneagan


double has a correspondence between compareTo and comparison operators, it's just special cased for NaN, right? That wouldn't prevent adding operators to Comparable.

The reason DateTime is an issue is that it conflates a moment in time with a calendar/timzone representation thereof. I presume two separate classes was deemed too complicated.

I still think it would be nice to have the Quantity interface/mixin above and for num/int/double/Duration to implement it.

@floitschG
Copy link
Contributor

Since double special cases for some values (NaN and -0.0) there is no bijection. If a sort-function expects a Comparable it would not be clear how it sorts.

Yes. DateTime is actually a ZonedDateTime. However splitting DateTime into different entities wouldn't make this issue easier. As a developer you want to work with ZonedDateTimes since that's the most useful. Otherwise you could just use the millisecondsSinceEpoch.

@DartBot DartBot added Type-Defect area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. closed-not-planned Closed as we don't intend to take action on the reported issue labels May 4, 2013
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. closed-not-planned Closed as we don't intend to take action on the reported issue
Projects
None yet
Development

No branches or pull requests

4 participants