www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.learn - writing iterators without code duplication. inout?

reply "pompei2" <pompei2 gmail.com> writes:
Hello.

I want to add the option to iterate objects of my class using 
foreach. I need them to be iterable as view-only const and as 
mutable too. I would prefer to iterate using the "return a 
delegate" but if that's not possible, ranges are fine too. Also, 
I'd prefer a template-less solution over a templated one.


This is what I have, which works but has severe code duplication. 
I hoped inout would help me here, but I just can't figure it out. 
I also gave a try to ranges, but same thing again: I can only get 
it to work if I define my things twice.

(syntax highlight for the coming month: 
http://pastebin.com/TNmWWgsj)


    import std.conv, std.stdio;

    class Container
    {
        this(int from, int to)
        {
            while(from <= to) {
                _arr ~= from;
                from++;
            }
        }

        // FIXME: severe code duplication for const/nonconst/ref
        //        and I'm not even trying immutable yet
        int delegate(int delegate(ref int)) doIter()
        {
            writeln("calling non-const");

            int doTheIter(int delegate(ref int) dg)
            {
                int result = 0;
                foreach(ref i ; this._arr) {
                    result = dg(i);
                    if(result)
                        break;
                }
                return result;
            }

            return &doTheIter;
        }

        int delegate(int delegate(ref int)) doIter() const
        {
            writeln("calling const");

            int doTheIter(int delegate(ref int) dg)
            {
                int result = 0;
                foreach(i ; this._arr) {
                    result = dg(i);
                    if(result)
                        break;
                }
                return result;
            }

            return &doTheIter;
        }

        int[] _arr;
    }

    int main(string[] args)
    {
        Container c = new Container(1, 9);
        const Container cc = c;

        foreach(ref e ; c.doIter()) {
            writeln(e);
            e++;
        }

        foreach(e ; cc.doIter()) {
            writeln(e);
        }

        return 0;
    }
Dec 21 2011
next sibling parent Trass3r <un known.com> writes:
Can't really answer your original question, but
1. Why don't you use opApply?
2. Why do you use ref int even in the const version?
3. You could also use alias this to allow iteration, don't know if that's  
what you want in general though.
Dec 21 2011
prev sibling next sibling parent Timon Gehr <timon.gehr gmx.ch> writes:
On 12/21/2011 04:54 PM, pompei2 wrote:
 Hello.

 I want to add the option to iterate objects of my class using foreach. I
 need them to be iterable as view-only const and as mutable too. I would
 prefer to iterate using the "return a delegate" but if that's not
 possible, ranges are fine too. Also, I'd prefer a template-less solution
 over a templated one.


 This is what I have, which works but has severe code duplication. I
 hoped inout would help me here, but I just can't figure it out. I also
 gave a try to ranges, but same thing again: I can only get it to work if
 I define my things twice.

 (syntax highlight for the coming month: http://pastebin.com/TNmWWgsj)


 import std.conv, std.stdio;

 class Container
 {
 this(int from, int to)
 {
 while(from <= to) {
 _arr ~= from;
 from++;
 }
 }

 // FIXME: severe code duplication for const/nonconst/ref
 // and I'm not even trying immutable yet
 int delegate(int delegate(ref int)) doIter()
 {
 writeln("calling non-const");

 int doTheIter(int delegate(ref int) dg)
 {
 int result = 0;
 foreach(ref i ; this._arr) {
 result = dg(i);
 if(result)
 break;
 }
 return result;
 }

 return &doTheIter;
 }

 int delegate(int delegate(ref int)) doIter() const
 {
 writeln("calling const");

 int doTheIter(int delegate(ref int) dg)
 {
 int result = 0;
 foreach(i ; this._arr) {
 result = dg(i);
 if(result)
 break;
 }
 return result;
 }

 return &doTheIter;
 }

 int[] _arr;
 }

 int main(string[] args)
 {
 Container c = new Container(1, 9);
 const Container cc = c;

 foreach(ref e ; c.doIter()) {
 writeln(e);
 e++;
 }

 foreach(e ; cc.doIter()) {
 writeln(e);
 }

 return 0;
 }

Just remove the non-const overload. const member functions work with mutable, immutable and const receivers.
Dec 21 2011
prev sibling next sibling parent "pompei2" <pompei2 gmail.com> writes:
 Just remove the non-const overload. const member functions work 
 with mutable, immutable and const receivers.

Doing this, it compiles but it doesn't do what it should in two ways: 1. I leave the code as-is. It compiles but "e" in the first foreach loop in "main" is a copy, not a reference. This shows by the second loop still displaying the original values, not the +1 values. 2. To fix 1., I can change the "i" in the foreach loop in "doTheIter" to "ref i", which will allow the first foreach loop in "main" to successfully modify the array elements. But now, add the line "e++;" into the second loop and it still compiles! Change it to "ref e" (still in the second loop) and add a third loop and it actually does modify the elements! It shouldn't be allowed to, as "cc" is const and const is supposed to be transitive. I'm not even using a cast. So either this is not the solution, or I'm doing it wrong.
Dec 21 2011
prev sibling next sibling parent "Jakob Ovrum" <jakobovrum gmail.com> writes:
On Wednesday, 21 December 2011 at 16:07:55 UTC, Timon Gehr wrote:
 Just remove the non-const overload. const member functions work 
 with mutable, immutable and const receivers.

The const version does not allow using 'ref' when iterating.
Dec 21 2011
prev sibling next sibling parent "pompei2" <pompei2 gmail.com> writes:
On Wednesday, 21 December 2011 at 16:05:24 UTC, Trass3r wrote:
 Can't really answer your original question, but
 1. Why don't you use opApply?
 2. Why do you use ref int even in the const version?
 3. You could also use alias this to allow iteration, don't know 
 if that's what you want in general though.

1&3: Because there are different things in my class to iterate over. Think foreach(p ; obj.properties()) and foreach(c ; obj.components()). (I know, I can make those property so I don't need the ().) 2. Because if not, it says: Error: cannot implicitly convert expression (__foreachbody1315) of type int delegate(ref int) to int delegate(int)
Dec 21 2011
prev sibling next sibling parent "Jakob Ovrum" <jakobovrum gmail.com> writes:
On Wednesday, 21 December 2011 at 16:31:01 UTC, Jakob Ovrum wrote:
 On Wednesday, 21 December 2011 at 16:07:55 UTC, Timon Gehr 
 wrote:
 Just remove the non-const overload. const member functions 
 work with mutable, immutable and const receivers.

The const version does not allow using 'ref' when iterating.

In light of pompei2's recent reply; the const version *should not* allow using 'ref' when iterating.
Dec 21 2011
prev sibling next sibling parent travert phare.normalesup.org (Christophe) writes:
"pompei2" , dans le message (digitalmars.D.learn:31164), a écrit :
 This is what I have, which works but has severe code duplication. 
 I hoped inout would help me here, but I just can't figure it out. 
 I also gave a try to ranges, but same thing again: I can only get 
 it to work if I define my things twice.

It's not optimal, and there is an ugly cast, but maybe this is a suitable workarrond for you : int delegate(int delegate(ref int)) doIter() const { return (int delegate(ref int) dg) { cast(typeof(this))(this).doIter()((ref int i) { int copy = i; dg(copy); }); } } Until int delegate(ref inout int) opApply() inout; and int delegate(int delegate(ref inout int)) doIter() inout; are made to work. (I actually don't know if there is any obstacles to do this).
Dec 21 2011
prev sibling next sibling parent "pompei2" <pompei2 gmail.com> writes:
 int delegate(int delegate(ref int)) doIter() const
   {
74      return (int delegate(ref int) dg)
       {
         cast(typeof(this))(this).doIter()
77            (
78              (ref int i)
               {
                 int copy = i; dg(copy);
               }
           );
       }	
   }

I see what you are doing there. Unfortunately, the innermost delegate (the one doing the copy trick) somehow got system attribute, which leads to the following compile errors (I added line numbers and reordered your code in the citation above): constiter.d(78): Error: cannot implicitly convert expression (__dgliteral2) of type void delegate(ref int i) system to int delegate(ref int) constiter.d(77): Error: cast has no effect in expression (cast(const(Container))this.doIter()((__error))) constiter.d(74): Error: cannot implicitly convert expression (__dgliteral1) of type void delegate(int delegate(ref int) dg) system to int delegate(int delegate(ref int)) Where does this system come from? How do I get rid of it?
 Until
 int delegate(ref inout int) opApply() inout;
 and
 int delegate(int delegate(ref inout int)) doIter() inout;
 are made to work. (I actually don't know if there is any 
 obstacles to do this).

Can you point me to the bugreport so I can vote for it? I'm not sure if it is #4838 or if they are unrelated.
Dec 22 2011
prev sibling next sibling parent Andrew Wiley <wiley.andrew.j gmail.com> writes:
On Thu, Dec 22, 2011 at 12:04 AM, pompei2 <pompei2 gmail.com> wrote:
 int delegate(int delegate(ref int)) doIter() const
 =A0{
 74 =A0 =A0 =A0return (int delegate(ref int) dg)
 =A0 =A0 =A0{
 =A0 =A0 =A0 =A0cast(typeof(this))(this).doIter()
 77 =A0 =A0 =A0 =A0 =A0 =A0(
 78 =A0 =A0 =A0 =A0 =A0 =A0 =A0(ref int i)

 =A0 =A0 =A0 =A0 =A0 =A0 =A0{
 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0int copy =3D i; dg(copy);
 =A0 =A0 =A0 =A0 =A0 =A0 =A0}
 =A0 =A0 =A0 =A0 =A0);
 =A0 =A0 =A0}
 =A0}

I see what you are doing there. Unfortunately, the innermost delegate (th=

 one doing the copy trick) somehow got  system attribute, which leads to t=

 following compile errors (I added line numbers and reordered your code in
 the citation above):

 constiter.d(78): Error: cannot implicitly convert expression (__dgliteral=

 of type void delegate(ref int i)  system to int delegate(ref int)
 constiter.d(77): Error: cast has no effect in expression
 (cast(const(Container))this.doIter()((__error)))
 constiter.d(74): Error: cannot implicitly convert expression (__dgliteral=

 of type void delegate(int delegate(ref int) dg)  system to int delegate(i=

 delegate(ref int))

 Where does this  system come from? How do I get rid of it?

Actually, it looks like your issue is here: constiter.d(78): Error: cannot implicitly convert expression (__dgliteral2) of type *void* delegate(ref int i) system to int delegate(ref int) void delegate(ref int) !=3D int delegate(ref int)
Dec 22 2011
prev sibling next sibling parent "Steven Schveighoffer" <schveiguy yahoo.com> writes:
On Wed, 21 Dec 2011 11:34:18 -0500, pompei2 <pompei2 gmail.com> wrote:

 On Wednesday, 21 December 2011 at 16:05:24 UTC, Trass3r wrote:
 Can't really answer your original question, but
 1. Why don't you use opApply?
 2. Why do you use ref int even in the const version?
 3. You could also use alias this to allow iteration, don't know if  
 that's what you want in general though.

1&3: Because there are different things in my class to iterate over. Think foreach(p ; obj.properties()) and foreach(c ; obj.components()). (I know, I can make those property so I don't need the ().) 2. Because if not, it says: Error: cannot implicitly convert expression (__foreachbody1315) of type int delegate(ref int) to int delegate(int)

Two of my most wished for bugs: http://d.puremagic.com/issues/show_bug.cgi?id=2443 http://d.puremagic.com/issues/show_bug.cgi?id=2498 Looks like 2443 has been fixed in head! -Steve
Jan 07 2012
prev sibling parent "Steven Schveighoffer" <schveiguy yahoo.com> writes:
On Wed, 21 Dec 2011 10:54:06 -0500, pompei2 <pompei2 gmail.com> wrote:

 Hello.

 I want to add the option to iterate objects of my class using foreach. I  
 need them to be iterable as view-only const and as mutable too. I would  
 prefer to iterate using the "return a delegate" but if that's not  
 possible, ranges are fine too. Also, I'd prefer a template-less solution  
 over a templated one.


 This is what I have, which works but has severe code duplication. I  
 hoped inout would help me here, but I just can't figure it out. I also  
 gave a try to ranges, but same thing again: I can only get it to work if  
 I define my things twice.

inout cannot be used here, because inout is const within an inout function. This means you cannot modify data while in the context of the function, and the foreach delegate is called within the context of the function. Having dealt with foreach in my container lib, I can tell you, it's not ideal. I'm hoping to have some way to do tail-const ranges in the future, which should help with code duplication. -Steve
Jan 07 2012