Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Fix behaviour of TreeSubSet Min and Max #16771

Merged
stephentoub merged 3 commits into from
Mar 7, 2017
Merged

Fix behaviour of TreeSubSet Min and Max #16771

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cbafaf8
Fix behaviour of TreeSubSet Min and Max
mjp41 Mar 6, 2017
0315d3f
Added a more exhaustive test of Min/Max on TreeSubSet.
mjp41 Mar 7, 2017
0e302d8
Code review feedback.
mjp41 Mar 7, 2017
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
59 changes: 59 additions & 0 deletions src/System.Collections/src/System/Collections/Generic/SortedSet.TreeSubSet.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,65 @@ internal override bool IsWithinRange(T item)
return comp >= 0;
}

internal override T MinInternal
{
get
{
Node current = _root;
T result = default(T);

while (current != null)
{

int comp = _lBoundActive ? Comparer.Compare(_min, current.Item) : -1;
if (comp == 1)
{
current = current.Right;
}
else
{
result = current.Item;
if (comp == 0)
{
break;
}
current = current.Left;
}
}

return result;
}
}
Copy link
Member

@stephentoub stephentoub Mar 7, 2017

Choose a reason for hiding this comment

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

Nits:

  • Could you move the opening braces from the end of the lines to the beginning of the next?
  • Could you move the else to its own line?
  • Could you surround the break with braces (on their own lines)?

Same in MaxInternal below.


internal override T MaxInternal
{
get
{
Node current = _root;
T result = default(T);

while (current != null)
{
int comp = _uBoundActive ? Comparer.Compare(_max, current.Item) : 1;
if (comp == -1)
{
current = current.Left;
}
else
{
result = current.Item;
if (comp == 0)
{
break;
}
current = current.Right;
}
}

return result;
}
}

internal override bool InOrderTreeWalk(TreeWalkPredicate<T> action)
{
VersionCheck();
Expand Down
8 changes: 6 additions & 2 deletions src/System.Collections/src/System/Collections/Generic/SortedSet.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1575,7 +1575,9 @@ public int RemoveWhere(Predicate<T> match)

#region ISorted members

public T Min
public T Min => MinInternal;

internal virtual T MinInternal
{
get
{
Expand All @@ -1594,7 +1596,9 @@ public T Min
}
}

public T Max
public T Max => MaxInternal;

internal virtual T MaxInternal
{
get
{
Expand Down
25 changes: 24 additions & 1 deletion src/System.Collections/tests/Generic/SortedSet/SortedSet.Generic.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ protected override int CreateT(int seed)

protected override bool DefaultValueAllowed => true;

[ActiveIssue(16760)]
[Fact]
public void SortedSet_Generic_GetViewBetween_MinMax()
{
Expand Down Expand Up @@ -63,6 +62,30 @@ public void SortedSet_Generic_IntersectWith_SupersetEnumerableWithDups()

Assert.Equal(new[] { 3, 5, 7 }, set);
}

[Fact]
public void SortedSet_Generic_GetViewBetween_MinMax_Exhaustive()
{
var set = (SortedSet<int>)CreateSortedSet(new[] { 7, 11, 3, 1, 5, 9, 13 }, 7, 7);
for (int i = 0; i < 14; i++)
{
for (int j = i; j < 14; j ++)
{
SortedSet<int> view = set.GetViewBetween(i, j);

if (j < i || (j == i && i % 2 == 0) )
{
Assert.Equal(default(int), view.Min);
Assert.Equal(default(int), view.Max);
}
else
{
Assert.Equal(i + ((i+1) % 2), view.Min);
Assert.Equal(j - ((j+1) % 2), view.Max);
}
}
}
}
}

public class SortedSet_Generic_Tests_int_With_NullComparer : SortedSet_Generic_Tests_int
Expand Down