Skip to content
Merged
Changes from all commits
Commits
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
207 changes: 207 additions & 0 deletions std/algorithm/searching.d
Original file line number Diff line number Diff line change
Expand Up @@ -3544,6 +3544,213 @@ unittest
assert(minPos!("a[0] < b[0]")(b) == [ [2, 4], [4], [4] ]);
}

/**
Computes the index of the first occurrence of `range`'s minimum element.

Params:
pred = The ordering predicate to use to determine the minimum element.
range = The input range to search.

Complexity: O(n)
Exactly `n - 1` comparisons are needed.

Returns:
The index of the first encounter of the minimum element in `range`. If the
`range` is empty, -1 is returned.

See_Also:
$(REF min, std,algorithm,comparison), $(LREF minCount), $(LREF minElement), $(LREF minPos)
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

how about

See_Also:
    $(REF min, std,algorithm,comparison), $(LREF minCount), $(LREF minElement), $(LREF minPos)

sizediff_t minIndex(alias pred = "a < b", Range)(Range range)
if (isForwardRange!Range && !isInfinite!Range &&
is(typeof(binaryFun!pred(range.front, range.front))))
Copy link
Member

Choose a reason for hiding this comment

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

is(typeof(binaryFun!pred(range.front, range.front))) == bool)

{
if (range.empty) return -1;
Copy link
Contributor

Choose a reason for hiding this comment

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

For efficiency in release mode, minElement (using private extremum) assumes the range is not empty:

    assert(!r.empty, "r is an empty range");

In that case we can use size_t instead of ptrdiff_t. (Forgot to mention this yesterday).

Copy link
Member

Choose a reason for hiding this comment

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

I think we're good here.

Copy link
Contributor

Choose a reason for hiding this comment

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

This runs counter to the behavior of most range functions in Phobos. Normally an empty range is rejected outright with an assert rather than given a special value.

I would change this to assert(!range.empty);

Copy link
Member

Choose a reason for hiding this comment

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

I would say we can return -1 here

Copy link
Member

Choose a reason for hiding this comment

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

I struggled with this for a bit. We have minElement and minCount, which has no reasonable way to report an empty range, so it just explodes. We then have minPos, which elegantly - :) - sidesteps the issue by returning a potentially empty range. This kind of leaves minIndex on its own. There are other functions that must deal with empty ranges, can you guys enumerate a few?

Copy link
Member

@9il 9il Dec 8, 2016

Choose a reason for hiding this comment

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

There are other functions that must deal with empty ranges, can you guys enumerate a few?

reduce and fold, #4937 replaces enforce for them with asserts

Copy link
Member

Choose a reason for hiding this comment

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

@9il thx. @JackStouffer other precedents we may draw from?


sizediff_t minPos = 0;

static if (isRandomAccessRange!Range && hasLength!Range)
{
foreach (i; 1 .. range.length)
{
if (binaryFun!pred(range[i], range[minPos]))
{
minPos = i;
}
}
}
else
{
sizediff_t curPos = 0;
Unqual!(typeof(range.front)) min = range.front;
for (range.popFront(); !range.empty; range.popFront())
{
++curPos;
if (binaryFun!pred(range.front, min))
{
min = range.front;
minPos = curPos;
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add a special path for arrays - unfortunately it will be a lot faster.
Take a look at extremum

return minPos;
}

///
@safe pure nothrow unittest
{
int[] a = [2, 3, 4, 1, 2, 4, 1, 1, 2];

// Minimum is 1 and first occurs in position 3
assert(a.minIndex == 3);
// Get maximum index with minIndex
assert(a.minIndex!"a > b" == 2);

// Range is empty, so return value is -1
int[] b;
assert(b.minIndex == -1);

// Works with more custom types
struct Dog { int age; }
Dog[] dogs = [Dog(10), Dog(5), Dog(15)];
assert(dogs.minIndex!"a.age < b.age" == 1);
}

@safe pure unittest
{
// should work with const
const(int)[] immArr = [2, 1, 3];
assert(immArr.minIndex == 1);

// Works for const ranges too
const int[] c = [2, 5, 4, 1, 2, 3];
assert(c.minIndex == 3);

// should work with immutable
immutable(int)[] immArr2 = [2, 1, 3];
assert(immArr2.minIndex == 1);

// with strings
assert(["b", "a", "c"].minIndex == 1);

// infinite range
import std.range : cycle;
static assert(!__traits(compiles, cycle([1]).minIndex));

// with all dummy ranges
import std.internal.test.dummyrange : AllDummyRanges;
foreach (DummyType; AllDummyRanges)
{
static if (isForwardRange!DummyType && !isInfinite!DummyType)
{
DummyType d;
d.arr = [5, 3, 7, 2, 1, 4];
assert(d.minIndex == 4);

d.arr = [];
assert(d.minIndex == -1);
}
}
}

@nogc @safe nothrow pure unittest
{
static immutable arr = [7, 3, 8, 2, 1, 4];
assert(arr.minIndex == 4);

static immutable arr2d = [[1, 3], [3, 9], [4, 2]];
assert(arr2d.minIndex!"a[1] < b[1]" == 2);
}

/**
Computes the index of the first occurrence of `range`'s maximum element.

Complexity: O(n)
Exactly `n - 1` comparisons are needed.

Params:
pred = The ordering predicate to use to determine the maximum element.
range = The input range to search.

Returns:
The index of the first encounter of the maximum in `range`. If the
`range` is empty, -1 is returned.

See_Also:
$(REF max, std,algorithm,comparison), $(LREF maxCount), $(LREF maxElement), $(LREF maxPos)
*/
sizediff_t maxIndex(alias pred = "a < b", Range)(Range range)
if (isInputRange!Range && !isInfinite!Range &&
is(typeof(binaryFun!pred(range.front, range.front))))
{
return range.minIndex!((a, b) => binaryFun!pred(b, a));
}

///
@safe pure nothrow unittest
{
// Maximum is 4 and first occurs in position 2
int[] a = [2, 3, 4, 1, 2, 4, 1, 1, 2];
assert(a.maxIndex == 2);

Copy link
Contributor

Choose a reason for hiding this comment

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

please add the emptyness example here as well

// Empty range
int[] b;
assert(b.maxIndex == -1);

// Works with more custom types
struct Dog { int age; }
Dog[] dogs = [Dog(10), Dog(15), Dog(5)];
assert(dogs.maxIndex!"a.age < b.age" == 1);
}

@safe pure unittest
{
// should work with const
const(int)[] immArr = [5, 1, 3];
assert(immArr.maxIndex == 0);

// Works for const ranges too
const int[] c = [2, 5, 4, 1, 2, 3];
assert(c.maxIndex == 1);
Copy link
Member

Choose a reason for hiding this comment

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

The tests should also test more than the default string lambda.

  • lambda
  • another string lambda

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Is it ok now?

Copy link
Member

Choose a reason for hiding this comment

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

yes, thank you



// should work with immutable
immutable(int)[] immArr2 = [2, 1, 3];
assert(immArr2.maxIndex == 2);

// with strings
assert(["b", "a", "c"].maxIndex == 2);

// infinite range
import std.range : cycle;
static assert(!__traits(compiles, cycle([1]).maxIndex));

// with all dummy ranges
import std.internal.test.dummyrange : AllDummyRanges;
foreach (DummyType; AllDummyRanges)
{
static if (isForwardRange!DummyType && !isInfinite!DummyType)
{
DummyType d;

d.arr = [5, 3, 7, 2, 1, 4];
assert(d.maxIndex == 2);

d.arr = [];
assert(d.maxIndex == -1);
}
}
}

@nogc @safe nothrow pure unittest
{
static immutable arr = [7, 3, 8, 2, 1, 4];
assert(arr.maxIndex == 2);

static immutable arr2d = [[1, 3], [3, 9], [4, 2]];
assert(arr2d.maxIndex!"a[1] < b[1]" == 1);
}

Copy link
Contributor

@wilzbach wilzbach Dec 13, 2016

Choose a reason for hiding this comment

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

Please add additional tests for

  1. @nogc (with static immutable)
  2. different input ranges (have a look in std.range for DummyRange -> e.g. have a look here or here
  3. test with strings
  4. Infinite ranges (static assert)
  5. Immutable ranges
  6. With common element types (classes, structs, tuples)

/**
Skip over the initial portion of the first given range that matches the second
range, or do nothing if there is no match.
Expand Down