www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.learn - Lexicographical object comparison by selected members of a struct

reply =?UTF-8?Q?Ali_=c3=87ehreli?= <acehreli yahoo.com> writes:
Sometimes I need comparison operators that should consider only some 
members of a struct:

struct S {
   int year;         // Primary member
   int month;        // Secondary member

   string[] values;  // Irrelevant
}

I've been using the laziest tuple+tupleof solution in some of my structs:

   bool opEquals(const typeof(this) that) {
     import std.typecons : tuple;

     return tuple(this.tupleof).opEquals(tuple(that.tupleof));
   }

   // Similar for opCmp.

That is repetitive, expensive at run time, and does not satisfy a 
requirement: Some members (like 'values' above) should not be considered 
during comparison.

I am sure you must have come up with similar solutions like the 
following code, or perhaps this whole thing exists in Phobos but I just 
wrote it today... for fun... :) (I think it exists somewhere but I could 
not find it.)  The code is not used in production yet but it should 
allow me to do the following:

struct S {
   int year;         // Primary member
   int month;        // Secondary member

   string[] values;  // Irrelevant

   mixin MemberwiseComparison!(year, month);  // 'values' excluded; good
}

What do you think?

I have a feeling that e.g. extracting member names from the strings like 
"this.foo" with the help of findSplitAfter(members[i].stringof) is 
pretty suspect. Could I do better?

At least the generated assembly is minimal to my eyes. (Much better than 
my lazy tuple+tupleof complication! :) )

// This helper function is defined outside of MemberwiseComparison
// because we don't want to mixin such a member function into user
// structs.
private string memberName(string thisName) {
   import std.algorithm : findSplitAfter;
   import std.exception : enforce;
   import std.range : empty;
   import std.format : format;

   const found = thisName.findSplitAfter(".");
   enforce(!found[0].empty,
           format!`Failed to find '.' in "%s"`(thisName));

   return found[1];
}

unittest {
   assert(memberName("this.foo") == "foo");

   // It should throw when no '.' is found.
   import std.exception;
   assertThrown(memberName("woo/hoo"));
}

// Mixes in opEquals() and opCmp() that perform lexicographical
// comparisons according to the order of 'members'.
mixin template MemberwiseComparison(members...) {
   bool opEquals(const typeof(this) that) const {

     // A nested function that makes a comparison expression.
     string makeCode(string name) {
       import std.format : format;

       return format!q{
         const a = this.%s;
         const b = that.%s;

         if (a != b) {
           // Early return at first mismatch
           return false;
         }
       }(name, name);
     }

     // Comparison code per member, which would potentially return an
     // early 'false' result.
     static foreach (i; 0 .. members.length) {{
       mixin (makeCode(memberName(members[i].stringof)));
     }}

     // There was no mismatch if we got here.
     return true;
   }

   int opCmp(const typeof(this) that) const {

     // A nested function that makes a comparison expression.
     string makeCode(string name) {
       import std.format : format;

       return format!q{
         const a = this.%s;
         const b = that.%s;

         if (a < b) {
           // 'this' is before; early return
           return -1;
         }

         if (a > b) {
           // 'this' is after; early return
           return  1;
         }
       }(name, name);
     }

     // Comparison code per member, which would potentially return an
     // early before or after decision.
     static foreach (i; 0 .. members.length) {{
       mixin (makeCode(memberName(members[i].stringof)));
     }}

     // Neither 'this' or 'that' was before if we got here.
     return 0;
   }
}

unittest {
   struct S {
     int i;
     double d;
     string s;

     // Only i and d are considered for this type.
     mixin MemberwiseComparison!(i, d);
   }

   // The order is decided by the first member.
   assert(S(1, 2) < S(2, 2));
   assert(S(3, 2) > S(2, 2));

   // The order is decided by the second member.
   assert(S(1, 2) < S(1, 3));
   assert(S(1, 2) > S(1, 1));

   // Objects are equal regardless of the third member.
   assert(S(7, 42, "hello") == S(7, 42, "world"));
}

void main() {
}

Ali
Aug 20 2021
parent reply Alexandru Ermicioi <alexandru.ermicioi gmail.com> writes:
On Saturday, 21 August 2021 at 04:34:46 UTC, Ali Çehreli wrote:

 ...


Consider __traits(identifier, member) instead. This one should 
return member name as string, removing the need of memberName 
function.

Also you could have an annotation  Equality.Include for example, 
and make mixin scan fields for it and include only those fields 
and methods that have it into compariosn and hashing.

Another problem that may arise is storage in local variable of 
fields before comparison, it might introduce unnecessary copy.

Regards,
Alexandru.
Aug 20 2021
parent reply Alexandru Ermicioi <alexandru.ermicioi gmail.com> writes:
On Saturday, 21 August 2021 at 05:34:59 UTC, Alexandru Ermicioi 
wrote:

 ...


Also there is no need for mixing string code here. You can get 
the field using __traits(getMember, this, member).
Aug 20 2021
parent reply =?UTF-8?Q?Ali_=c3=87ehreli?= <acehreli yahoo.com> writes:
On 8/20/21 10:37 PM, Alexandru Ermicioi wrote:

 On Saturday, 21 August 2021 at 05:34:59 UTC, Alexandru Ermicioi wrote:

 ...


 
 Also there is no need for mixing string code here. You can get the field 
 using __traits(getMember, this, member).


Cool! Much better. :)

I could not do

   static foreach (member; members) {{
     // ...
   }}

So I am happy with iterating over 0 .. members.length.

// Mixes in opEquals() and opCmp() that perform lexicographical
// comparisons according to the order of 'members'.
mixin template MemberwiseComparison(members...) {
   bool opEquals(const typeof(this) that) const {

     // Comparison code per member, which would potentially return an
     // early 'false' result.
     static foreach (i; 0 .. members.length) {{
       // mixin (makeCode(__traits(identifier, members[i])));
         enum ident = __traits(identifier, members[i]);

         if (__traits(getMember, this, ident) != __traits(getMember, 
that, ident)) {
           return false;
         }
     }}

     // There was no mismatch if we got here.
     return true;
   }

   int opCmp(const typeof(this) that) const {

     // Comparison code per member, which would potentially return an
     // early before or after decision.
     static foreach (i; 0 .. members.length) {{
       enum ident = __traits(identifier, members[i]);

       if (__traits(getMember, this, ident) < __traits(getMember, that, 
ident)) {
         return -1;
       }
       if (__traits(getMember, this, ident) > __traits(getMember, that, 
ident)) {
         return 1;
       }
     }}

     // Neither 'this' nor 'that' was before if we got here.
     return 0;
   }
}

Ali
Aug 20 2021
parent reply Tejas <notrealemail gmail.com> writes:
On Saturday, 21 August 2021 at 06:03:33 UTC, Ali Çehreli wrote:

 On 8/20/21 10:37 PM, Alexandru Ermicioi wrote:

 [...]


 Cool! Much better. :)

 I could not do

 [...]


Did you use that double curly bracket in `static foreach` so that 
you don't get error for declaring stuff inside `static foreach` ?
Aug 20 2021
parent reply =?UTF-8?Q?Ali_=c3=87ehreli?= <acehreli yahoo.com> writes:
On 8/20/21 11:19 PM, Tejas wrote:

 On Saturday, 21 August 2021 at 06:03:33 UTC, Ali =C3=87ehreli wrote:

 On 8/20/21 10:37 PM, Alexandru Ermicioi wrote:

 [...]


 Cool! Much better. :)

 I could not do

 [...]


=20
 Did you use that double curly bracket in `static foreach` so that you=20
 don't get error for declaring stuff inside `static foreach` ?


Yes. 'static foreach' does not introduce scope, which can be pretty=20
useful. For example, one can define functions at module scope.

The subtle differences between 'static foreach' and '(compile-time)=20
foreach' can be surprising. For example, I don't understand why I can't u=
se

   foreach (i; 0 .. members.length) {
     enum ident =3D __traits(identifier, members[i]);
     // ...
   }

Error: variable `i` cannot be read at compile time.

I think it should be. :) members.length is compile-time; so, 'i' should=20
be compile time.

And then, why can't 'static foreach' iterate over 'members'?

   static foreach (member; members) {{
     // ...
   }}

Error: value of `this` is not known at compile time

Hm? Well, I am happy that there is always a path through. :)

Ali
Aug 20 2021
parent reply Tejas <notrealemail gmail.com> writes:
On Saturday, 21 August 2021 at 06:58:47 UTC, Ali Çehreli wrote:

 On 8/20/21 11:19 PM, Tejas wrote:

 [...]


 Yes. 'static foreach' does not introduce scope, which can be 
 pretty useful. For example, one can define functions at module 
 scope.

 The subtle differences between 'static foreach' and 
 '(compile-time) foreach' can be surprising. For example, I 
 don't understand why I can't use

   foreach (i; 0 .. members.length) {
     enum ident = __traits(identifier, members[i]);
     // ...
   }

 Error: variable `i` cannot be read at compile time.

 I think it should be. :) members.length is compile-time; so, 
 'i' should be compile time.

 And then, why can't 'static foreach' iterate over 'members'?

   static foreach (member; members) {{
     // ...
   }}

 Error: value of `this` is not known at compile time

 Hm? Well, I am happy that there is always a path through. :)

 Ali


Maybe it has something to do with that compile-time vs 
compile-time thing by quickfur on the dlang wiki.

I was more impressed that you found that hack in the first place
Aug 21 2021
parent reply =?UTF-8?Q?Ali_=c3=87ehreli?= <acehreli yahoo.com> writes:
On 8/21/21 1:31 AM, Tejas wrote:


 I was more impressed that you found that hack in the first place


I can't take credit. :) 'static foreach' had that difference since its 
inception. The spec says "If a new scope is desired for each expansion, 
use another set of braces:"

   https://dlang.org/spec/version.html#staticforeach

Ali
Aug 21 2021
parent Tejas <notrealemail gmail.com> writes:
On Saturday, 21 August 2021 at 13:45:59 UTC, Ali Çehreli wrote:

 On 8/21/21 1:31 AM, Tejas wrote:


 I was more impressed that you found that hack in the first


 place

 I can't take credit. :) 'static foreach' had that difference 
 since its inception. The spec says "If a new scope is desired 
 for each expansion, use another set of braces:"

   https://dlang.org/spec/version.html#staticforeach

 Ali


I'm an inattentive dunce ;_;
Aug 21 2021