Deque impl.

[monarch_dodra]

On Tuesday, 29 January 2013 at 19:44:23 UTC, Robert Schadek wrote:
> Hi,
>
> I have a Deque implementation that I really like. I would like 
> to get
> some comments on it.
> http://dpaste.1azy.net/4bf119e7 dpaste does not run the 
> unittests
> successfully. I don't
> know why. I tested on a x32 as well as x64 machine and it works 
> on both.
>
> Best Regards
> Robert

Appart from the fact you seem to have copy pasted your code 
twice, here are a quick comments, interface wise. I didn't check 
anything implementation-wise

1. When you put a unittest inside the definition of a 
class/struct, it gets run for every instantiation of said class, 
and has access to "T". This is good for writing generic tests for 
the current T type. This can be useful to test things that depend 
on T, such as making sure some functions are indeed safe, or 
whatnot.

If you are testing a specific implementation, then move it out of 
the struct.

For example:
//----
import std.stdio;

struct S(T)
{
     unittest
     {
         //Tests S!T for every T.
         writeln("Unittest: S!", T.stringof);
     }
}

unittest
{
     //Write a specific S!int test here, for example
     writeln("Unittest: Global");
}

void main()
{
     alias A = S!int;    //Force S!int tests
     alias B = S!double; //Force S!int tests
}
//----

"rdmd -unittest main.d" produces:
//----
Unittest: Global
Unittest: S!int
Unittest: S!double
//----

2. Nitpick: I'd personally prefer you use the term "Range" for 
your iterator. Iterator != Range. You are confusing me. Also, I'd 
consider nesting the iterator definition inside your deque.

3. Nitpick: Don't use parenthesis for single arg templates:
No:  "Deque!(int)"
yes: "Deque!int"

4. Why does your iterator have two template parameters? Seems it 
is always "T" and "Deque!T". Why bother with the second parameter 
at all? If you place it inside your Deque implementation, then it 
becomes parameter-less (appart from the implicit T of course). 
The added advantage is that it becomes a "Deque!int.Iterator" 
type. This is good if you ever write "List", and don't want to 
have the name "Iterator" clash...

5. Users expect "opSlice()" to produce an actual range. You are 
using it to deep copy the deck. This goes against your comment in 
regards to slicing an iterator. Just have your "opSlice()" do the 
same as what "range()" does, and get rid of "range()". This is 
the usage I'd expect:

//----
Deque!int    de = ... ;
Iterator!int it = de[];
writeln(it);
//----

I'll review the actual deque implementation tomorrow.