Skip to content

improvements/fixes for count countUntil #951

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

Merged
merged 6 commits into from
Dec 13, 2012
Merged

improvements/fixes for count countUntil #951

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
1b7db2d
improvements/fixes for count countUntil
monarchdodra Nov 15, 2012
3dffa6c
more improvements in cout/countUntil
monarchdodra Nov 19, 2012
a98a93f
fixing countUntil vs indexOf issue
monarchdodra Nov 16, 2012
0a3b7be
Quick splitter countUntil fix
monarchdodra Nov 17, 2012
076c853
indexOf: More details
monarchdodra Nov 17, 2012
20d77cb
Changes to count[Until] as suggested by review
monarchdodra Nov 22, 2012
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
194 changes: 141 additions & 53 deletions std/algorithm.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1924,7 +1924,7 @@ assert(equal(splitter(a, 0), [ [], [1] ]));
*/
auto splitter(Range, Separator)(Range r, Separator s)
if (is(typeof(ElementType!Range.init == Separator.init))
&& (hasSlicing!Range || isNarrowString!Range))
&& ((hasSlicing!Range && hasLength!Range) || isNarrowString!Range))
{
static struct Result
{
Expand All @@ -1941,8 +1941,8 @@ if (is(typeof(ElementType!Range.init == Separator.init))
{
static IndexType lastIndexOf(Range haystack, Separator needle)
{
immutable index = countUntil(retro(haystack), needle);
return (index == -1) ? -1 : haystack.length - 1 - index;
auto r = haystack.retro().find(needle);
Copy link
Member

Choose a reason for hiding this comment

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

what about eliminating them parens :o)

Copy link
Member

Choose a reason for hiding this comment

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

retro is a function, not a property, so I'd argue that the parens should be there. But even if you don't agree with that, I'm not sure that it will even compile without them right now, because Phobos is compiled with -property. Certainly, if -property were fully implemented as originally intended it wouldn't, but I can't remember what -property does and doesn't manage to check right now.

Copy link
Member

Choose a reason for hiding this comment

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

OK. I was just alluding to the fact that there's a strong current to allow paren-less invocations for parameterless functions.

return r.retro().length - 1;
}
}

Expand Down Expand Up @@ -1970,8 +1970,8 @@ if (is(typeof(ElementType!Range.init == Separator.init))
assert(!empty);
if (_frontLength == _unComputed)
{
_frontLength = countUntil(_input, _separator);
if (_frontLength == -1) _frontLength = _input.length;
auto r = _input.find(_separator);
_frontLength = _input.length - r.length;
}
return _input[0 .. _frontLength];
}
Expand Down Expand Up @@ -4006,35 +4006,45 @@ unittest
assert(countUntil("hello world", "world") == 6);
assert(countUntil("hello world", 'r') == 8);
assert(countUntil("hello world", "programming") == -1);
assert(countUntil("日本語", "本語") == 1);
assert(countUntil("日本語", '語') == 2);
assert(countUntil("日本語", "五") == -1);
assert(countUntil("日本語", '五') == -1);
assert(countUntil([0, 7, 12, 22, 9], [12, 22]) == 2);
assert(countUntil([0, 7, 12, 22, 9], 9) == 4);
assert(countUntil!"a > b"([0, 7, 12, 22, 9], 20) == 3);
--------------------
+/
ptrdiff_t countUntil(alias pred = "a == b", R, N)(R haystack, N needle)
if (is(typeof(startsWith!pred(haystack, needle))))
ptrdiff_t countUntil(alias pred = "a == b", R1, R2)(R1 haystack, R2 needle)
if (isForwardRange!R1 && isForwardRange!R2 &&
is(typeof(binaryFun!pred(haystack.front, needle.front)) : bool))
{
static if (isNarrowString!R)
typeof(return) result;
static if (hasLength!R1) //Note: Narrow strings don't have length.
{
// Narrow strings are handled a bit differently
auto length = haystack.length;
for (; !haystack.empty; haystack.popFront())
{
if (startsWith!pred(haystack, needle))
{
return length - haystack.length;
}
}
//Delegate to find. Find is very efficient
//We save haystack, but we don't care for needle
auto r2 = find!pred(haystack.save, needle);
if (!r2.empty) return cast(typeof(return)) (haystack.length - r2.length);
}
else
{
typeof(return) result;
for (; !haystack.empty; ++result, haystack.popFront())
{
if (startsWith!pred(haystack, needle)) return result;
}
//Default case, slower route doing startsWith iteration
for ( ; !haystack.empty ; ++result, haystack.popFront() )
if (startsWith!pred(haystack.save, needle.save)) return result;
}
return -1;

//Because of @@@8804@@@: Avoids both "unreachable code" or "no return statement"
static if (isInfinite!R1) assert(0);
else return -1;
}
/// ditto
ptrdiff_t countUntil(alias pred = "a == b", R, N)(R haystack, N needle)
if (isInputRange!R &&
is(typeof(binaryFun!pred(haystack.front, needle)) : bool))
{
bool pred2(ElementType!R a) { return binaryFun!pred(a, needle); }
return countUntil!pred2(haystack);
}

//Verify Examples.
Expand All @@ -4043,6 +4053,10 @@ unittest
assert(countUntil("hello world", "world") == 6);
assert(countUntil("hello world", 'r') == 8);
assert(countUntil("hello world", "programming") == -1);
assert(countUntil("日本語", "本語") == 1);
assert(countUntil("日本語", '語') == 2);
assert(countUntil("日本語", "五") == -1);
assert(countUntil("日本語", '五') == -1);
assert(countUntil([0, 7, 12, 22, 9], [12, 22]) == 2);
assert(countUntil([0, 7, 12, 22, 9], 9) == 4);
assert(countUntil!"a > b"([0, 7, 12, 22, 9], 20) == 3);
Expand All @@ -4060,29 +4074,46 @@ assert(countUntil!"a > 20"([0, 7, 12, 22, 9]) == 3);
--------------------
+/
ptrdiff_t countUntil(alias pred, R)(R haystack)
if (isForwardRange!R && is(typeof(unaryFun!pred(haystack.front)) == bool))
if (isInputRange!R &&
is(typeof(unaryFun!pred(haystack.front)) : bool))
{
static if (isNarrowString!R)
typeof(return) i;
static if (isRandomAccessRange!R)
{
// Narrow strings are handled a bit differently
auto length = haystack.length;
for (; !haystack.empty; haystack.popFront())
//Optimized RA implementation. Since we want to count *and* iterate at
//the same time, it is more efficient this way.
static if (hasLength!R)
{
if (unaryFun!pred(haystack.front))
{
return length - haystack.length;
}
immutable len = cast(typeof(return)) haystack.length;
for ( ; i < len ; ++i )
if (unaryFun!pred(haystack[i])) return i;
Copy link
Member

Choose a reason for hiding this comment

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

Why isn't len declared in the for loop?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'd say mostly out of habit: The first parameter of the for loop is usually used to initialize i, so the initialization of len = r.length is almost always outside of the loop.

As far as :

for ( auto len = cast(typeof(return)) haystack.length ; i < len ; ++i)

I find it distils what the loop is actually doing.

I'd say it's a matter of taste, but if you have a good reason for the change (you usually do), I wouldn't mind hearing it.

Copy link
Member

Choose a reason for hiding this comment

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

In general, variables should be restricted to the minimal scope possible. It reduces the odds of misusing them, and makes it clearer what they pertain to (and if they hold resources, then those resources will be released that much more quickly - especially if the type is on the stack with a destructor rather than living on the GC heap). So, in general, declaring a variable inside a for loop is preferred when it does not need to exist outside of that for loop. In this particular case, the scope that the for loop is in terminates immediately after the loop, so there isn't much impact, but that's often not the case. So stylistically, I'm very much in favor of putting the variable inside the loop, but its impact here is minimal, so it's up to you.

By the way, I'd advise using immutable rather than auto in cases like this one, because then the compiler knows that the variable won't change and is more like to be able to optimize stuff relating to it (particularly since it's an integral type and definitely won't have problems being immutable, unlike many user-defined types). However, given that the only place that len is used is the loop's condition, it will likely have zero impact here. It's still a good habit to get into though IMHO.

}
else //if (isInfinite!R)
{
for ( ; ; ++i )
if (unaryFun!pred(haystack[i])) return i;
}
}
else
else static if (hasLength!R)
{
typeof(return) result;
for (; !haystack.empty; ++result, haystack.popFront())
//For those odd ranges that have a length, but aren't RA.
//It is faster to quick find, and then compare the lengths
auto r2 = find!pred(haystack.save);
if (!r2.empty) return cast(typeof(return)) (haystack.length - r2.length);
}
else //Everything else
{
alias ElementType!R T; //For narrow strings forces dchar iteration
foreach (T elem; haystack)
{
if (unaryFun!pred(haystack.front)) return result;
if (unaryFun!pred(elem)) return i;
++i;
}
}
return -1;

//Because of @@@8804@@@: Avoids both "unreachable code" or "no return statement"
static if (isInfinite!R) assert(0);
else return -1;
}

//Verify Examples.
Expand All @@ -4092,14 +4123,51 @@ unittest
assert(countUntil!(std.ascii.isDigit)("hello world") == -1);
assert(countUntil!"a > 20"([0, 7, 12, 22, 9]) == 3);
}
unittest
{
// References
{
// input
ReferenceInputRange!int r;
r = new ReferenceInputRange!int([0, 1, 2, 3, 4, 5, 6]);
assert(r.countUntil(3) == 3);
r = new ReferenceInputRange!int([0, 1, 2, 3, 4, 5, 6]);
assert(r.countUntil(7) == -1);
}
{
// forward
auto r = new ReferenceForwardRange!int([0, 1, 2, 3, 4, 5, 6]);
assert(r.save.countUntil([3, 4]) == 3);
assert(r.save.countUntil(3) == 3);
assert(r.save.countUntil([3, 7]) == -1);
assert(r.save.countUntil(7) == -1);
}
{
// infinite forward
auto r = new ReferenceInfiniteForwardRange!int(0);
assert(r.save.countUntil([3, 4]) == 3);
assert(r.save.countUntil(3) == 3);
}
}

/**
* $(RED Deprecated. It will be removed in January 2013.
* Please use $(LREF countUntil) instead.)
* Currently defaults to $(LREF countUntil) instead.)
*
* Same as $(D countUntil). This symbol has been deprecated
* because it is easily confused with the homonym function
* Not to be confused with its homonym function
* in $(D std.string).
*
* Please use $(D std.string.indexOf) if you wish to find
* the index of a character in a string.
*
* Otherwise, please use $(D std.string.countUntil) to find
* an element's logical position in a range.
*
* Example:
* --------
* assert(std.string.indexOf("日本語", '本') == 3);
* assert(std.algorithm.countUntil("日本語", '本') == 1);
* --------
*/
deprecated ptrdiff_t indexOf(alias pred = "a == b", R1, R2)(R1 haystack, R2 needle)
if (is(typeof(startsWith!pred(haystack, needle))))
Expand Down Expand Up @@ -4952,6 +5020,9 @@ $(D 2).
The third version counts the elements for which $(D pred(x)) is $(D
true). Performs $(BIGOH r.length) evaluations of $(D pred).

Note: Regardless of the overload, $(D count) will not accept
infinite ranges for $(D haystack).

Example:
----
// count elements in range
Expand All @@ -4962,15 +5033,18 @@ assert(count!("a > b")(a, 2) == 5);
assert(count("abcadfabf", "ab") == 2);
assert(count("ababab", "abab") == 1);
assert(count("ababab", "abx") == 0);
// fuzzy count range in range
assert(count!"std.uni.toLower(a) == std.uni.toLower(b)"("AbcAdFaBf", "ab") == 2);
// count predicate in range
assert(count!("a > 1")(a) == 8);
----
*/
size_t count(alias pred = "a == b", Range, E)(Range r, E value)
if (isInputRange!Range && is(typeof(binaryFun!pred(r.front, value)) == bool))
size_t count(alias pred = "a == b", Range, E)(Range haystack, E needle)
if (isInputRange!Range && !isInfinite!Range &&
is(typeof(binaryFun!pred(haystack.front, needle)) : bool))
{
bool pred2(ElementType!(Range) a) { return binaryFun!pred(a, value); }
return count!(pred2)(r);
bool pred2(ElementType!Range a) { return binaryFun!pred(a, needle); }
return count!pred2(haystack);
}

unittest
Expand Down Expand Up @@ -5001,31 +5075,44 @@ unittest

/// Ditto
size_t count(alias pred = "a == b", R1, R2)(R1 haystack, R2 needle)
if (isInputRange!R1 && isForwardRange!R2 && is(typeof(binaryFun!pred(haystack, needle)) == bool))
if (isForwardRange!R1 && !isInfinite!R1 &&
isForwardRange!R2 &&
is(typeof(binaryFun!pred(haystack.front, needle.front)) : bool))
{
enforce(!needle.empty, "Cannot count occurrences of an empty range");
size_t result;
for (; findSkip!pred(haystack, needle); ++result)
static if (isInfinite!R2)
{
//Note: This is the special case of looking for an infinite inside a finite...
//"How many instances of the Fibonacci sequence can you count in [1, 2, 3]?" - "None."
return 0;
}
else
{
size_t result;
//Note: haystack is not saved, because findskip is designed to modify it
for ( ; findSkip!pred(haystack, needle.save) ; ++result)
{}
return result;
}
return result;
}

unittest
{
assert(count("abcadfabf", "ab") == 2);
assert(count("ababab", "abab") == 1);
assert(count("ababab", "abx") == 0);
assert(count!"std.uni.toLower(a) == std.uni.toLower(b)"("AbcAdFaBf", "ab") == 2);
}

/// Ditto
size_t count(alias pred = "true", Range)(Range r) if (isInputRange!(Range))
size_t count(alias pred = "true", R)(R haystack)
if (isInputRange!R && !isInfinite!R &&
is(typeof(unaryFun!pred(haystack.front)) : bool))
{
size_t result;
for (; !r.empty; r.popFront())
{
if (unaryFun!pred(r.front)) ++result;
}
alias ElementType!R T; //For narrow strings forces dchar iteration
foreach (T elem; haystack)
if (unaryFun!pred(elem)) ++result;
return result;
}

Expand All @@ -5035,6 +5122,7 @@ unittest
writeln("unittest @", __FILE__, ":", __LINE__, " done.");
int[] a = [ 1, 2, 4, 3, 2, 5, 3, 2, 4 ];
assert(count!("a == 3")(a) == 2);
assert(count("日本語") == 3);
}

// balancedParens
Expand Down
2 changes: 1 addition & 1 deletion std/path.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2682,7 +2682,7 @@ string expandTilde(string inputPath)

// Extract username, searching for path separator.
string username;
auto last_char = std.algorithm.countUntil(path, dirSeparator[0]);
auto last_char = std.string.indexOf(path, dirSeparator[0]);

if (last_char == -1)
{
Expand Down