Skip to content
Closed
Show file tree
Hide file tree
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
1 change: 1 addition & 0 deletions std/algorithm/package.d
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ $(TR
$(SUBREF searching, maxCount)
$(SUBREF searching, minElement)
$(SUBREF searching, maxElement)
$(SUBREF searching, minmaxElement)
$(SUBREF searching, minIndex)
$(SUBREF searching, maxIndex)
$(SUBREF searching, minPos)
Expand Down
191 changes: 191 additions & 0 deletions std/algorithm/searching.d
Original file line number Diff line number Diff line change
Expand Up @@ -3551,6 +3551,197 @@ if (isInputRange!Range && !isInfinite!Range &&
assert(arr2d.maxElement!"a[1]" == arr2d[1]);
}

/**
Iterates the passed range and selects the min and max element with `less`.
If the extreme element occurs multiple time, the first occurrence will be
returned.
This function is more efficient than calling both $(LREF minElement) and
$(LREF maxElement).
Copy link
Contributor

@JackStouffer JackStouffer Apr 29, 2016

Choose a reason for hiding this comment

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

Could go into a little more detail here. I think you should add something like

This function is more efficient than calling both $(LREF minElement) and $(LREF maxElement) for one range because this function only requires one scan of the range, whereas the former takes two. Also, calling both $(LREF minElement) and $(LREF maxElement) on the same range would require it to be a forward range.

This would help people understand this function's benefits more clearly.

Copy link
Member

Choose a reason for hiding this comment

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

More precisely, we should provide guarantees similar to the C++ version - per http://en.cppreference.com/w/cpp/algorithm/minmax_element: "At most max(floor(3/2(N−1)), 0) applications of the predicate, where N = std::distance(first, last)."


Params:
map = custom accessor for the comparison key
selector = custom mapping for the extrema selection
minSeed = custom seed to use as initial element for min
maxSeed = custom seed to use as initial element for min
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo here, should be "custom seed to use as initial element for max"

r = Range from which the extreme value will be selected

Returns:
The extreme value according to `map` and `selector` of the passed-in values.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be something like

A $(XREF Tuple, typecons) of the extreme values according to map and selector of the range.


See_Also:
Copy link
Contributor

Choose a reason for hiding this comment

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

Should also provide a link to the C++ version

Copy link
Contributor

@nordlow nordlow May 1, 2016

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

I agree, this won't stop decision to pull.

$(LREF minElement), $(LREF maxElement)
*/
auto minmaxElement(alias map = "a", alias selector = "a < b", Range)(Range r)
Copy link
Member

Choose a reason for hiding this comment

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

Can you define two overloads so there's no default for map?

Copy link
Member

Choose a reason for hiding this comment

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

Reiterating this request.

if (isInputRange!Range && !isInfinite!Range)
in
{
assert(!r.empty, "r is an empty range");
}
body
{
alias Element = ElementType!Range;
Unqual!Element seed = r.front;
r.popFront();
return minmaxElement!(map, selector)(r, seed, seed);
}

/// ditto
auto minmaxElement(alias map = "a", alias selector = "a < b", Range,
RangeElementType = ElementType!Range)
(Range r, RangeElementType minSeed, RangeElementType maxSeed)
if (isInputRange!Range && !isInfinite!Range &&
!is(CommonType!(ElementType!Range, RangeElementType) == void))
{
alias mapFun = unaryFun!map;
alias selectorFun = binaryFun!selector;

Copy link
Member

Choose a reason for hiding this comment

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

assert(!selector(maxSeed, minSeed));

alias Element = ElementType!Range;
alias CommonElement = CommonType!(Element, RangeElementType);
alias MapType = Unqual!(typeof(mapFun(CommonElement.init)));

Unqual!CommonElement minElement = minSeed;
Unqual!CommonElement maxElement = maxSeed;

MapType minElementMapped = mapFun(minElement);
MapType maxElementMapped = mapFun(maxElement);

alias minmaxTuple = Tuple!(Unqual!CommonElement, "min", Unqual!CommonElement, "max");
Copy link
Contributor

Choose a reason for hiding this comment

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

Add unittests for access via members min and max. Otherwise LGTM.


static if (isRandomAccessRange!Range && hasLength!Range)
{
foreach (const i; 0 .. r.length)
{
MapType mapElement = mapFun(r[i]);
if (selectorFun(mapElement, minElementMapped))
{
minElement = r[i];
minElementMapped = mapElement;
}
if (selectorFun(maxElementMapped, mapElement))
{
maxElement = r[i];
maxElementMapped = mapElement;
}
Copy link
Member

Choose a reason for hiding this comment

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

So here we should use the 3n/2 algorithm, even if technically slower for < and int. One great thing to do would be to specialize for this (and a few other) cases.

}
}
else
{
while (!r.empty)
{
MapType rawElement1 = r.front;
MapType mapElement1 = mapFun(rawElement1);
r.popFront();
// check if the range had an uneven amount of elements and thus has ended
if (r.empty)
Copy link
Member

Choose a reason for hiding this comment

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

BUG: must return at the end of this if.

{
if (selectorFun(mapElement1, minElementMapped))
{
minElement = rawElement1;
minElementMapped = mapElement1;
}
if (selectorFun(maxElementMapped, mapElement1))
{
maxElement = rawElement1;
maxElementMapped = mapElement1;
}
}
MapType rawElement2 = r.front;
MapType mapElement2 = mapFun(rawElement1);
r.popFront();

if (selectorFun(mapElement1, mapElement2))
{
if (selectorFun(mapElement1, minElementMapped))
{
minElement = rawElement1;
minElementMapped = mapElement1;
}
if (selectorFun(maxElementMapped, mapElement2))
{
maxElement = rawElement2;
maxElementMapped = mapElement2;
}
}
else
{
if (selectorFun(mapElement2, minElementMapped))
{
maxElement = rawElement2;
minElementMapped = mapElement2;
}
if (selectorFun(maxElementMapped, mapElement1))
{
minElement = rawElement1;
maxElementMapped = mapElement1;
}

}
}
}
return minmaxTuple(minElement, maxElement);
}

///
@safe pure unittest
{
import std.range: enumerate;
import std.stdio;

assert([2, 1, 4, 3].minmaxElement == tuple(1, 4));

//// allows to get the index of an element too
assert([5, 3, 7, 9].enumerate.minmaxElement!"a.value" == tuple(tuple(1, 3), tuple(3, 9)));

// any custom accessor can be passed
assert([[0, 4], [1, 2]].minmaxElement!"a[1]" == tuple([1, 2], [0, 4]));

// can be seeded
int[] arr;
assert(arr.minmaxElement(1, 2) == tuple(1, 2));
}

@safe pure unittest
{
import std.range: enumerate, iota;
// supports mapping
assert([3, 4, 5, 1, 2].enumerate.minmaxElement!"a.value" ==
tuple(tuple(3, 1), tuple(2, 5)));
Copy link
Contributor

Choose a reason for hiding this comment

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

Indent looks incorrect.

assert([5, 2, 4].enumerate.minmaxElement!"a.value" ==
tuple(tuple(1, 2), tuple(0, 5)));
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto


// forward ranges
assert(iota(1, 5).minmaxElement() == tuple(1, 4));
assert(iota(2, 5).enumerate.minmaxElement!"a.value" ==
tuple(tuple(0, 2), tuple(2, 4)));
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto


// should work with const
const(int)[] immArr = [2, 1, 3];
assert(immArr.minmaxElement == tuple(1, 3));

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

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

// with all dummy ranges
import std.internal.test.dummyrange;
foreach (DummyType; AllDummyRanges)
{
DummyType d;
assert(d.minmaxElement == tuple(1, 10));
}
}

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

static immutable arr2d = [[1, 9], [3, 1], [2, 12], [4, 2]];
assert(arr2d.minmaxElement!"a[1]" == tuple(arr2d[1], arr2d[2]));
}
// minPos
/**
Computes a subrange of `range` starting at the first occurrence of `range`'s
Expand Down