www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - Operators overloading

reply "matovitch" <camille.brugel laposte.net> writes:
Hello from France everyone ! (I hope my english will be 
readable...)

Yesterday, I discover D and that was really really great. I am 
far from being a guru (it seems there are a lot here ;-) but I 
foud the language clear and expressive the documentation is very 
well done futhermore dmd is incredibly fast and the logs are pure 
awesomness (a gcc command often doesn't fit on my shell). I know 
you are waiting for a 'but' and there is one : it's about 
operators overloading. First, but that kind of ok with me, I 
don't like the syntax :

	const Vertex opBinary(string op)(const ref Vertex vertex)
		if (op == "+" || op == "-")
	{
		Vertex result = *this;
		mixin("result" ~ op ~ "= vertex;");
		return result;
	}

I don't like the 'if' outside and the condition could be really 
long : if (op == "+" || op == "-" || op == "*" || op == "/"). 
Okay, putting the if inside is quite easy :

	const Vertex opBinary(string op)(const ref Vertex vertex)	
	{
		static if 	if (op == "+" || op == "-")
		{
			Vertex result = *this;
			mixin("result" ~ op ~ "= vertex;");
			return result;
		}
	}

But...I'd like to define cross product :

	const T opBinary(string op)(const ref Vertex vertex)
	{
		static if (op == "*")
		{
			T result = 0;
			foreach(i; 0..vertex_size)
				result += coordinate[i] * vertex.coordinate[i];
			return result;
		}
	}

The compiler is not so great here and doesn't distinguish the two 
opBinary by their signatures. Why ? Before looking at the 
documentation again I tried something like that :
	
	const Vertex opBinary(string op : ["+", "-"])(const ref Vertex 
vertex)	
	{
		{
			Vertex result = *this;
			mixin("result" ~ op ~ "= vertex;");
			return result;
		}
	}

	const T opBinary(string op : "*")(const ref Vertex vertex)
	{
		
		T result = 0;
		foreach(i; 0..vertex_size)
			result += coordinate[i] * vertex.coordinate[i];
		return result;
	}

The second one is okay, but the fisrt one isn't correct. I think 
this kind of template specialization over a list could be great 
with mixins and static if/switch. What do you think ? May be 
there is an other way to do what I want...
May 19 2013
next sibling parent reply Jonathan M Davis <jmdavisProg gmx.com> writes:
On Sunday, May 19, 2013 09:32:09 matovitch wrote:

 Hello from France everyone ! (I hope my english will be
 readable...)
 
 Yesterday, I discover D and that was really really great. I am
 far from being a guru (it seems there are a lot here ;-) but I
 foud the language clear and expressive the documentation is very
 well done futhermore dmd is incredibly fast and the logs are pure
 awesomness (a gcc command often doesn't fit on my shell). I know
 you are waiting for a 'but' and there is one : it's about
 operators overloading. First, but that kind of ok with me, I
 don't like the syntax :
 
 	const Vertex opBinary(string op)(const ref Vertex vertex)
 		if (op == "+" || op == "-")
 	{
 		Vertex result = *this;
 		mixin("result" ~ op ~ "= vertex;");
 		return result;
 	}
 
 I don't like the 'if' outside and the condition could be really
 long : if (op == "+" || op == "-" || op == "*" || op == "/").
 Okay, putting the if inside is quite easy :
 
 	const Vertex opBinary(string op)(const ref Vertex vertex)
 	{
 		static if 	if (op == "+" || op == "-")
 		{
 			Vertex result = *this;
 			mixin("result" ~ op ~ "= vertex;");
 			return result;
 		}
 	}
 
 But...I'd like to define cross product :
 
 	const T opBinary(string op)(const ref Vertex vertex)
 	{
 		static if (op == "*")
 		{
 			T result = 0;
 			foreach(i; 0..vertex_size)
 				result += coordinate[i] * vertex.coordinate[i];
 			return result;
 		}
 	}
 
 The compiler is not so great here and doesn't distinguish the two
 opBinary by their signatures. Why ? Before looking at the
 documentation again I tried something like that :
 
 	const Vertex opBinary(string op : ["+", "-"])(const ref Vertex
 vertex)
 	{
 		{
 			Vertex result = *this;
 			mixin("result" ~ op ~ "= vertex;");
 			return result;
 		}
 	}
 
 	const T opBinary(string op : "*")(const ref Vertex vertex)
 	{
 
 		T result = 0;
 		foreach(i; 0..vertex_size)
 			result += coordinate[i] * vertex.coordinate[i];
 		return result;
 	}
 
 The second one is okay, but the fisrt one isn't correct. I think
 this kind of template specialization over a list could be great
 with mixins and static if/switch. What do you think ? May be
 there is an other way to do what I want...


If you want to distinguish between overloads of templated functions based on 
template parameters, template constraints are the correct way to do it. 
Nothing inside of the function - including static ifs - is considered in 
overloading. By the time the compiler even looks at that code, the overload 
has already been decided (or determined to be ambiguous). And what you can do 
directly in the template parameters to distinguish them - e.g. string op : "+" 
- is extremely limited. Much as you may not like it, the correct way to handle 
this is with template constraints. That's what they're for.

Now, there are ways to reduce their length - particularly if you're testing 
the same thing often. In that case, you could make an eponymous template for 
the test.

template isAddOrSub(string op)
{
    enum isAddOrSub = op == "+" || op == "-";
}

Then you can do

const Vertex opBinary(string op)(const ref Vertex vertex)
    if (isAddOrSub!op)
{...}

Now, in this particular case, I wouldn't think that it would be worth it. 
Which operators you need to use with a particular overload varies too much 
from type to type for an eponymous template like that to make much sense.

Another alternative would be to use a function like std.algorithm.equal except 
that it tested that one of the arguments was equal to the first one rather than 
just testing one argument against it. Something like

const Vertex opBinary(string op)(const ref Vertex vertex)
    if (isOneOf(op, "+", "-"))
{...}

Off the top of my head, I'm not aware of a function like that in the standard 
library, but I may just not being think of it, and it probably wouldn't be all 
that hard to write, at least if efficiency wasn't a major concern.

In general though, AFAIK, most people just do

const Vertex opBinary(string op)(const ref Vertex vertex)
    if (op == "+" || op == "-")
{...}

and don't have a problem with it.

- Jonathan M Davis
May 19 2013
next sibling parent "matovitch" <camille.brugel laposte.net> writes:
Fisrt, thank you for your clear answer.

On Sunday, 19 May 2013 at 09:04:23 UTC, Jonathan M Davis wrote:

 Nothing inside of the function - including static ifs - is 
 considered in overloading.


I understand but it isn't about something which is inside the 
function : it is about its signature.

I agree that sometimes it might be useful to use more than a list 
of or, so the idea of template specilization over a list is 
stupid and theses template constraints are useful (even if I 
don't like the syntax, I couldn't imagine a better one).
May 19 2013
prev sibling parent reply "bearophile" <bearophileHUGS lycos.com> writes:
Jonathan M Davis:


 Another alternative would be to use a function like 
 std.algorithm.equal except
 that it tested that one of the arguments was equal to the first 
 one rather than
 just testing one argument against it. Something like

 const Vertex opBinary(string op)(const ref Vertex vertex)
     if (isOneOf(op, "+", "-"))
 {...}

 Off the top of my head, I'm not aware of a function like that 
 in the standard
 library, but I may just not being think of it, and it probably 
 wouldn't be all
 that hard to write, at least if efficiency wasn't a major 
 concern.


It's named std.algorithm.canFind:

if (["+", "-"].canFind(op))

Bye,
bearophile
May 19 2013
parent Jonathan M Davis <jmdavisProg gmx.com> writes:
On Sunday, May 19, 2013 12:36:00 bearophile wrote:

 Jonathan M Davis:

 Another alternative would be to use a function like
 std.algorithm.equal except
 that it tested that one of the arguments was equal to the first
 one rather than
 just testing one argument against it. Something like
 
 const Vertex opBinary(string op)(const ref Vertex vertex)
 
     if (isOneOf(op, "+", "-"))
 
 {...}
 
 Off the top of my head, I'm not aware of a function like that
 in the standard
 library, but I may just not being think of it, and it probably
 wouldn't be all
 that hard to write, at least if efficiency wasn't a major
 concern.


 
 It's named std.algorithm.canFind:
 
 if (["+", "-"].canFind(op))


Ah, yes. I was thinking backwards. At first, I was thinking canFind(op, "+", 
"-"), which definitely doesn't do the right thing, and for some reason, it 
didn't occur to me to flip it. I've probably been up too long and should get to 
bed.

- Jonathan M Davis
May 19 2013
prev sibling next sibling parent reply Timon Gehr <timon.gehr gmx.ch> writes:
On 05/19/2013 09:32 AM, matovitch wrote:

 Okay, putting the if inside is quite easy :


It is not a valid transformation. This removes the condition from the 
signature.


      const Vertex opBinary(string op)(const ref Vertex vertex)
      {
          static /*[...]*/ if (op == "+" || op == "-")
          {
              Vertex result = *this;
              mixin("result" ~ op ~ "= vertex;");
              return result;
          }
      }

 But...I'd like to define cross product :

      const T opBinary(string op)(const ref Vertex vertex)
      {
          static if (op == "*")
          {
              T result = 0;
              foreach(i; 0..vertex_size)
                  result += coordinate[i] * vertex.coordinate[i];
              return result;
          }
      }

 The compiler is not so great here and doesn't distinguish the two
 opBinary by their signatures. Why ?


Because the two signatures are identical.

const T opBinary(string op)(const ref Vertex vertex);

Do this:

     const Vertex opBinary(string op)(const ref Vertex vertex)
     if (op == "+" || op == "-"){
         Vertex result = *this;
         mixin("result" ~ op ~ "= vertex;");
         return result;
     }

     const T opBinary(string op)(const ref Vertex vertex)
     if (op == "*"){
         T result = 0;
         foreach(i; 0..vertex_size)
             result += coordinate[i] * vertex.coordinate[i];
         return result;
     }
May 19 2013
parent "matovitch" <camille.brugel laposte.net> writes:
On Sunday, 19 May 2013 at 10:03:27 UTC, Timon Gehr wrote:

 On 05/19/2013 09:32 AM, matovitch wrote:

 Okay, putting the if inside is quite easy :


 It is not a valid transformation. This removes the condition 
 from the signature.


     const Vertex opBinary(string op)(const ref Vertex vertex)
     {
         static /*[...]*/ if (op == "+" || op == "-")
         {
             Vertex result = *this;
             mixin("result" ~ op ~ "= vertex;");
             return result;
         }
     }

 But...I'd like to define cross product :

     const T opBinary(string op)(const ref Vertex vertex)
     {
         static if (op == "*")
         {
             T result = 0;
             foreach(i; 0..vertex_size)
                 result += coordinate[i] * vertex.coordinate[i];
             return result;
         }
     }

 The compiler is not so great here and doesn't distinguish the 
 two
 opBinary by their signatures. Why ?


 Because the two signatures are identical.

 const T opBinary(string op)(const ref Vertex vertex);

 Do this:

     const Vertex opBinary(string op)(const ref Vertex vertex)
     if (op == "+" || op == "-"){
         Vertex result = *this;
         mixin("result" ~ op ~ "= vertex;");
         return result;
     }

     const T opBinary(string op)(const ref Vertex vertex)
     if (op == "*"){
         T result = 0;
         foreach(i; 0..vertex_size)
             result += coordinate[i] * vertex.coordinate[i];
         return result;
     }


Oh yes ! You are right...it takes a vertex in both cases. Thanks 
for losing your time with me. :-)
May 19 2013
prev sibling parent reply "Idan Arye" <GenericNPC gmail.com> writes:
On Sunday, 19 May 2013 at 07:32:11 UTC, matovitch wrote:

 	const Vertex opBinary(string op : ["+", "-"])(const ref Vertex 
 vertex)	
 	{
 		{
 			Vertex result = *this;
 			mixin("result" ~ op ~ "= vertex;");
 			return result;
 		}
 	}

 	const T opBinary(string op : "*")(const ref Vertex vertex)
 	{
 		
 		T result = 0;
 		foreach(i; 0..vertex_size)
 			result += coordinate[i] * vertex.coordinate[i];
 		return result;
 	}

 The second one is okay, but the fisrt one isn't correct. I 
 think this kind of template specialization over a list could be 
 great with mixins and static if/switch. What do you think ? May 
 be there is an other way to do what I want...


I think a better syntax would be:

     const Vertex opBinary(string op : "+", "-")(const ref Vertex 
vertex)

this will save a bit of ambiguity(`op` is a `string` 
specialization of `string[]`?)

At any rate, I'm not sure how useful it would be. Operator 
overloading code usually fit for a single operator - you usually 
write one for each operator:

	const Vertex opBinary(string op : "+")(const ref Vertex vertex)
	{
		Vertex result = this;
         result += vertex;
		return result;
	}
	const Vertex opBinary(string op : "-")(const ref Vertex vertex)
	{
		Vertex result = this;
         result -= vertex;
		return result;
	}

If you want to have several operators in the same method, you can 
afford the verbosity of:

     const Vertex opBinary(string op)(const ref Vertex vertex)
         if (__traits(compiles, mixin("this " ~ op ~ "= vertex;")))
     {
         Vertex result = *this;
         mixin("result" ~ op ~ "= vertex;");
         return result;
     }
May 19 2013
next sibling parent Jonathan M Davis <jmdavisProg gmx.com> writes:
On Sunday, May 19, 2013 12:41:52 Idan Arye wrote:

 At any rate, I'm not sure how useful it would be. Operator
 overloading code usually fit for a single operator - you usually
 write one for each operator:


Actually, it's usually considered good practice to use the same function for 
several operators and mixin the operator. In most cases, with arithmetic 
operators, that allows you to use one function for several operators and is 
the main reason why overloaded operators now use strings like they do rather 
than being opAdd, opSub, etc. like they used to be. Sometimes, it _is_ 
necessary to have an overload per operator, but usually that can be avoided.

- Jonathan M Davis
May 19 2013
prev sibling next sibling parent Timon Gehr <timon.gehr gmx.ch> writes:
On 05/19/2013 12:41 PM, Idan Arye wrote:

 ...

 I think a better syntax would be:

      const Vertex opBinary(string op : "+", "-")(const ref Vertex vertex)

 this will save a bit of ambiguity(`op` is a `string` specialization of
 `string[]`?)


It moves the ambiguity from analysis into the parser.


 At any rate, I'm not sure how useful it would be. Operator overloading
 code usually fit for a single operator - you usually write one for each
 operator:

      const Vertex opBinary(string op : "+")(const ref Vertex vertex)
      {
          Vertex result = this;
          result += vertex;
          return result;
      }
      const Vertex opBinary(string op : "-")(const ref Vertex vertex)
      {
          Vertex result = this;
          result -= vertex;
          return result;
      }


Your example shows how operator overloading code often fits for multiple 
operators.


 If you want to have several operators in the same method, you can afford
 the verbosity of:

      const Vertex opBinary(string op)(const ref Vertex vertex)
          if (__traits(compiles, mixin("this " ~ op ~ "= vertex;")))
      {
          Vertex result = /*[...]*/this;
          mixin("result" ~ op ~ "= vertex;");
          return result;
      }


This kind of template constraint may allow leaking of implementation 
details.

Eg. v.opBinary!".foo+"
May 19 2013
prev sibling parent "matovitch" <camille.brugel laposte.net> writes:
On Sunday, 19 May 2013 at 10:41:53 UTC, Idan Arye wrote:

 I think a better syntax would be:

     const Vertex opBinary(string op : "+", "-")(const ref 
 Vertex vertex)


I tried this first... Yes, it's a better syntax.
May 19 2013