www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.learn - User defined forward range - foreach initializes f.r.

reply Andre <andre s-e-a-p.de> writes:
Hi,

I have a template class "List" which should provide generic list 
functionality. In my use case, using a class instead a struct
is more comfortable.

Following unittest fails, although the forward range
implements the save method.

unittest
{
	auto intList = new List!int(1,2,3);
	assert(intList.length == 3);
	foreach(item; intList) {}
	assert(intList.length == 3);
}


class List(T)
{
	private T[] items;
	
	this(T[] items...)
	{
		this.items = items;
	}

	bool empty() const
     {
         return items.length == 0;
     }
	
	 property int length() const
	{
		return items.length;
	}

     void popFront()
     {
         items = items[1..$];
     }
	
	 property List!T save() const
	{
		return new List!T( cast(T[]) items);
	}

	T front() const
     {
         return opIndex(0);
     }
	
	T opIndex(size_t index) const
     {
         return cast(T) items[index];
     }
}

I already tried different thinks, like using dup or importing std.array.
But nothing helps, the unittest fails.
Why it fails?

Kind regards
André
Jun 13 2014
next sibling parent reply "H. S. Teoh via Digitalmars-d-learn" <digitalmars-d-learn puremagic.com> writes:
On Fri, Jun 13, 2014 at 08:14:11PM +0200, Andre via Digitalmars-d-learn wrote:

 Hi,
 
 I have a template class "List" which should provide generic list
 functionality. In my use case, using a class instead a struct
 is more comfortable.
 
 Following unittest fails, although the forward range
 implements the save method.


[...]

Generally, it's a bad idea to conflate a container with a range over its
contents. In this sense, built-in arrays are a very bad example, because
they show precisely this sort of conflation. But they get away with it
because of their hybrid value/reference semantics. Most containers,
however, don't have this sort of complex semantics, so they don't mix
very well with ranges directly.

It's much better to separate the container from a range over its
contents. So I wouldn't put the range API methods in the List class, but
implement an opSlice method that returns a struct that performs the list
iteration, that implements the range API. This allows you to write:

	auto myList = new List(...);
	foreach (item; myList[]) { // N.B., myList[] calls myList.opSlice()
		...
	}

Since the range struct returned by opSlice is separate from the
container itself, you don't have any risk of iteration also consuming
the container.


T

-- 
Let X be the set not defined by this sentence...
Jun 13 2014
parent Andre <andre s-e-a-p.de> writes:
Am 13.06.2014 20:22, schrieb H. S. Teoh via Digitalmars-d-learn:

 [...]

 Generally, it's a bad idea to conflate a container with a range over its
 contents. In this sense, built-in arrays are a very bad example, because
 they show precisely this sort of conflation. But they get away with it
 because of their hybrid value/reference semantics. Most containers,
 however, don't have this sort of complex semantics, so they don't mix
 very well with ranges directly.

 It's much better to separate the container from a range over its
 contents. So I wouldn't put the range API methods in the List class, but
 implement an opSlice method that returns a struct that performs the list
 iteration, that implements the range API. This allows you to write:

 	auto myList = new List(...);
 	foreach (item; myList[]) { // N.B., myList[] calls myList.opSlice()
 		...
 	}

 Since the range struct returned by opSlice is separate from the
 container itself, you don't have any risk of iteration also consuming
 the container.


 T


Thank you, this information is really helpful.

Kind regards
André
Jun 13 2014
prev sibling next sibling parent "monarch_dodra" <monarchdodra gmail.com> writes:
On Friday, 13 June 2014 at 18:14:10 UTC, Andre wrote:

 Hi,

 I have a template class "List" which should provide generic 
 list functionality. In my use case, using a class instead a 
 struct
 is more comfortable.

 Following unittest fails, although the forward range
 implements the save method.

 unittest
 {
 	auto intList = new List!int(1,2,3);
 	assert(intList.length == 3);
 	foreach(item; intList) {}
 	assert(intList.length == 3);
 }

 ...

 I already tried different thinks, like using dup or importing 
 std.array.
 But nothing helps, the unittest fails.
 Why it fails?

 Kind regards
 André


To add to what H.S. Teoh said, which is good advice, but doesn't 
quite explain why your unittest failed: Foreach takes a copy of 
your range, and then "consumes it". This may or may not have an 
effect on the original range. If you plan to use your range again 
after a call, you are supposed to *save* it:

unittest
{
	auto intList = new List!int(1,2,3);
	assert(intList.length == 3);
	foreach(item; intList.save) {} //HERE
	assert(intList.length == 3);
}

There.
Jun 13 2014
prev sibling parent reply =?UTF-8?B?QWxpIMOHZWhyZWxp?= <acehreli yahoo.com> writes:
I am making the following comment to have others confirm, as well as 
remind others about a potential problem.

On 06/13/2014 11:14 AM, Andre wrote:


 unittest
 {
      auto intList = new List!int(1,2,3);


[...]


 class List(T)
 {
      private T[] items;

      this(T[] items...)
      {
          this.items = items;
      }


Unrelated to your question, I think that is a bug because you are 
keeping a reference to a temporary array. The compiler will generate a 
temporary array for 1, 2, and 3 and then that array will disappear.

Am I remembering correctly?

If so, I would recommend against 'T[] items...' but use an explicit slice:

     this(T[] items)

The users will slightly be inconvenienced but the program will be correct.

Or, you can continue with 'T[] items...' and then have to take a copy of 
the argument:

         this.items = items.dup;    // copied

That is obviously slower when the user actually passed in a slice. 
That's why I think it is better to take an explicit slice as a parameter.

Ali
Jun 13 2014
parent "monarch_dodra" <monarchdodra gmail.com> writes:
On Friday, 13 June 2014 at 19:03:12 UTC, Ali Çehreli wrote:

 I am making the following comment to have others confirm, as 
 well as remind others about a potential problem.

 On 06/13/2014 11:14 AM, Andre wrote:


 unittest
 {
      auto intList = new List!int(1,2,3);


 [...]


 class List(T)
 {
      private T[] items;

      this(T[] items...)
      {
          this.items = items;
      }


 Unrelated to your question, I think that is a bug because you 
 are keeping a reference to a temporary array. The compiler will 
 generate a temporary array for 1, 2, and 3 and then that array 
 will disappear.

 Am I remembering correctly?

 If so, I would recommend against 'T[] items...' but use an 
 explicit slice:

     this(T[] items)

 The users will slightly be inconvenienced but the program will 
 be correct.

 Or, you can continue with 'T[] items...' and then have to take 
 a copy of the argument:

         this.items = items.dup;    // copied

 That is obviously slower when the user actually passed in a 
 slice. That's why I think it is better to take an explicit 
 slice as a parameter.

 Ali


Yes.

`fun(T)(T[]...)` is just both `fun(T...)(T t)` and `fun(T)(T[] 
t)` both "conveniently" packaged into one, but with twice the 
pitfalls.

I'd suggest avoiding it altogether. It's a non-feature (IMO).
Jun 13 2014