Skip to content

skipOver should have a single argument with pred version #5497

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.

Sign up for GitHub

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

Closed
JackStouffer wants to merge 1 commit into from
Closed

skipOver should have a single argument with pred version #5497

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 58 additions & 0 deletions std/algorithm/searching.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4026,6 +4026,64 @@ if (is(typeof(binaryFun!pred(r.front, e))) && isInputRange!R)
assert(!s2.skipOver('a'));
}

/**
* An overload of `skipOver` which takes a predicate. Skips over any elements
* from the start of the range that evaluate to `true`. Stops popping elements
* on the first element which `pred(r.front)` equals `false`.
*
* Params:
* pred = the test for each element
* r = the range to skip over
* Returns:
* `true` if elements were skipped over, `false` otherwise.
*/
bool skipOver(alias pred, R)(ref R r) if (ifTestable!(typeof(r.front), unaryFun!pred))
Copy link
Member

@andralex andralex Jun 29, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice. Please add but use /// ditto for the documentation and consolidate with the existing doc.

{
Copy link
Contributor

@wilzbach wilzbach Jun 20, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A) countUntil

import std.algorithm.iteration : filter;
import std.range;
return (&r).refRange.chain.countUntil!(x => !unaryFun!pred(x)) > 0;

B) find

import std.range;
return (&r).refRange.enumerate.find!(x => !unaryFun!pred(x.value)).front.index > 0;

Copy link
Member

@andralex andralex Jun 29, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do you mean that?

Copy link
Contributor

@wilzbach wilzbach Jun 29, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do you mean that?

It was just an FYI on how this functionality currently can be achieved. Of course, both ways are a bit clunky and won't be as fast as this implementation.

Copy link
Member

@CyberShadow CyberShadow Jun 29, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andralex If we could get find to be partially instantiatable with just the predicate then that solves a bunch of problems at once, without introducing new symbols. Unfortunately actually doing it is tricky - see comments below.

Copy link
Member

@andralex andralex Jul 7, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@CyberShadow I think making find instantiable with the predicate would help a lot of cases. That doesn't necesarily compete with skipOver which is useful for parsing.

bool ret = false;
Copy link

@ghost ghost Jun 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stop creating AST nodes for something that's done automatically (i.e default initialization).

Copy link
Member

@CyberShadow CyberShadow Jun 23, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is bad advice.

I can't find a quote at the moment, but Walter has been always saying that default initialization is not meant to provide useful default values, but to make using un-initialized variables deterministic and, wherever possible, an error. When possible, the default value of a type is an invalid one (NaN for floating-point types and '\xFF' for chars, which is an invalid UTF-8 code unit); the reason why most other types are initialized to zero is that they do not have an "invalid" value.

So, do initialize your variables explicitly (when you feel it makes sense to) and do not rely on their default value.

Copy link

@ghost ghost Jun 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What you didn't know is that the author of the PR is also the person who suggested to add a D-Scanner check for default initialization. My review comment couldn't be more legit.

Copy link
Member

@CyberShadow CyberShadow Jun 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not relevant. The pull request is for Phobos, not some personal repository of Jack's.


while (!r.empty && unaryFun!pred(r.front))
{
r.popFront;
ret = true;
}

return ret;
}

///
@safe pure unittest
{
import std.ascii : isWhite;

auto s1 = "\t\nHello";
assert(s1.skipOver!(isWhite));
assert(s1 == "Hello");

auto s2 = "Hello\t\n";
assert(s2.skipOver!(a => !isWhite(a)));
assert(s2 == "\t\n");

auto s3 = "Hello\t\n";
assert(!s3.skipOver!(isWhite));
assert(s3 == "Hello\t\n");
}

@system unittest
{
import std.algorithm.comparison : equal;
import std.ascii : isUpper;
import std.internal.test.dummyrange;

auto r1 = new ReferenceInputRange!int([1, 2, 3, 4, 5, 6]);
auto r2 = new ReferenceInputRange!dchar("Hello");

r1.skipOver!(a => a < 3);
assert(r1.equal([3, 4, 5, 6]));

r2.skipOver!(isUpper);
assert(r2.equal("ello"));
}

/**
Checks whether the given
$(REF_ALTTEXT input range, isInputRange, std,range,primitives) starts with (one
Expand Down