On 6/18/15 6:07 PM, ZombineDev wrote:
It's a nice start and I like in general how easy it is to integrate an
allocator to the design. The node ref-counting and disposal make this
almost a classic text-book example - it's that easy :)

Thanks for taking a look! Yah, I, too, was a tad surprised about the seamless dovetailing of things.

I have some comments/questions:
* I don't know if it's because this is WIP, but it also strange that the
range has empty and popFront primitives, but the Slist doesn't have them.

functional.SList should have empty but not popFront. The latter is mutating.

* More over, the distinction between the SList collection and it's range
is a bit blurry. They even have the same members. You have named your
module
std.[..]functional.[..], which makes me wonder if you actually need a
collection to operate with such lists. This idea reminds of programming
lisp where operations with lists are fluid and everything is floating
around and it just works without having to call
collection<T>.push_back(elem); a single-time. The difference being that
this is GC-free and with more deterministic behavior, but it still
resembles this productivity of "just get stuff done and don't bother me
with management".

Years ago when I chose the range primitives there was a choice between:

for (auto r = x[]; !r.empty; r.popFront) {
  ... use r.front ...
}

and

for (auto r = x[]; !r.empty; r = r.tail) {
  ... use r.front ...
}

The first won, partly because it's easier to implement efficiently. So now we have this one mutating operation that we need to mind. Because of it, we'll need functional containers to be distinct from their ranges.

* It's strange that the copy-constructor `this(this)` has reference
semantics (it adds reference to the list) and the assign-operator
(opAssign) has move semantics (it steals the contents of the victim on
assignment).
Edit: actually because Slist is a struct this line doesn't do anything
to the moved from list: http://dpaste.dzfl.pl/06e24fecd2da#line-93 ,
though to the comment in the function is a bit disorienting.

Yah, there's a little trick there that deserves an explanation - opAssign takes a SList by value, so the reference counting has already occurred at the caller side. So there's no extra need to do that, just move from it and leave it empty.

* In a couple of places like this:
http://dpaste.dzfl.pl/06e24fecd2da#line-181
you assert that the list is non-empty. Why don't you instead use in
contracts?

Well, it's briefer...


Andrei

Reply via email to