Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added range for MongoCursor #172

Closed
wants to merge 1 commit into from
Closed

Added range for MongoCursor #172

wants to merge 1 commit into from

Conversation

mihails-strasuns
Copy link
Contributor

One of enhancement I have long wanted - working with MongoCursor with InputRange interface. Allows for more functional way to process database data into final output with all std.algorithm & friends help.

Motivating example from tests:

coll.insert(["key1" : "value1"]);
coll.insert(["key1" : "value2"]);
coll.insert(["key1" : "value2"]);
auto data1 = coll.find(["key1" : "value1"]);
auto data2 = coll.find(["key1" : "value2"]);

import std.range, std.algorithm;
auto converted = map!
    (a => a[0].key1.get!string() ~ a[1].key1.get!string())(
        zip(data1.range, data2.range)
    );
assert(!converted.empty);
assert(converted.front == "value1value2");

@s-ludwig
Copy link
Member

Neat, always had that in mind but never had the immediate need to implement it.

One question, though - it seems to me that the range interface would fit nicely into MongoCursor itself. The opApply does not really imply that iteration works only once and the input range interface would be clearer there, I think.

@mihails-strasuns
Copy link
Contributor Author

It was my first naive approach but then second opApply overload functionality is lost : foreach(key, value; cursor).
I don't know how to do same via range interface.

@s-ludwig
Copy link
Member

Can't we just keep the second opApply in addition to the range interface?

@mihails-strasuns
Copy link
Contributor Author

Tried this too, got compilation error about not being able to select foreach signature. Was not in mood to get deep into specs so reverted to this solution. There is one advantage of keeping range separate though - usual phobos approach of separating containers from ranges. I.e. later we may change the implementation so that MongoCursor.range iteration won't consume items from inner struct and change it to ForwardRange.

@s-ludwig
Copy link
Member

But since MongoCursor itself consumes items during iteration (which surely is a good default regarding efficiency), it's more a range/range separation instead of a container/range. If a container/multiple iteration is needed, it think cursor.array() is an obvious solution that should be good enough for most cases.

It's just that only coll.find().map!(...) is much nicer than to always have to insert another range() call... I could also take look and see how range+index opApply can be made to work together - that just shouldn't be the only reason to separate the functionality into two basically equivalent entities. The spec seems to be a bit short on this.. range iteration isn't even mentioned in the foreach section, AFAICS.

@mihails-strasuns
Copy link
Contributor Author

@s-ludwig
It is, there is a separate section for range foreach in dlang.org documentation: http://dlang.org/statement.html#ForeachRangeStatement But I can't find one on foreach signature resolution order.

Anyway, I am perfectly fine with changing this back to MongoCursor as a range - just a bit busy to find out right now. Let me know if you will be first one to find out.

@s-ludwig
Copy link
Member

But that section is just about n .. m style "ranges" ;)

I'm also still busy with other things, but should have some time later today. I'll post as soon as I have something.

@mihails-strasuns
Copy link
Contributor Author

Hah, that is what you get when start coding 12h a day :)
Never mind, it was just my memory glitch. Looking forward to any updates.

@s-ludwig
Copy link
Member

Okay... it seems that once there is any opApply, the range interface is not taken into account for iteration anymore. The solution seems as simple as adding the range interface to MongoCursor and leave both opApplys there.

Do you mind if I just commit my changes and close the pull? Seems to be the fastest way.

@mihails-strasuns
Copy link
Contributor Author

Sure, please do. May be worth referencing commit hash in comment here for random wanderers.

s-ludwig added a commit that referenced this pull request Jan 29, 2013
…172.

The range interface is added directly to MongoCursor in the version instead
of introducing an extra range type. Original pull by Михаил Страшун (Dicebot).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants