Skip to content
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.

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

Fix behavior ObjectCollection for single item contains #59547

Merged
Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,9 @@ public void Clear()
}

public bool Contains(T item) =>
ReferenceEquals(item, _items) ||
(_size != 0 && _items is T[] items && Array.IndexOf(items, item, 0, _size) != -1);
_size <= 0 ? false :
_items is T o ? o.Equals(item) :
_items is T[] items && Array.IndexOf(items, item, 0, _size) != -1;

public void CopyTo(T[] array, int arrayIndex)
{
Expand All @@ -120,14 +121,16 @@ public void CopyTo(T[] array, int arrayIndex)

public bool Remove(T item)
{
if (ReferenceEquals(_items, item))
if (_items is T o)
{
_items = null;
_size = 0;
return true;
if (o.Equals(item))
{
_items = null;
_size = 0;
return true;
}
}

if (_items is T[] items)
else if (_items is T[] items)
{
int index = Array.IndexOf(items, item, 0, _size);
if (index != -1)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,74 @@ public void Ctor_ExecuteBothOverloads_MatchExpectation()
c.Add("value1");

Assert.Throws<InvalidOperationException>(() => { c.Add(null); });
}

[Fact]
public void ContainsAndRemove_UsesEqualitySemantics()
{
ObjectCollection<string> c = new ObjectCollection<string>();

string val1 = "value1";
string val1DifferentReference = "value" + 1;
Assert.NotSame(val1, val1DifferentReference);
Assert.Equal(val1, val1DifferentReference);

string val2 = "value2";
string val2DifferentReference = "value" + 2;
Assert.NotSame(val2, val2DifferentReference);
Assert.Equal(val2, val2DifferentReference);

string val3 = "value3";

// Start empty
Assert.Equal(0, c.Count);
Assert.False(c.Contains(val1));

// Single item
c.Add(val1);
Assert.Equal(1, c.Count);
Assert.True(c.Contains("value1"));
Assert.True(c.Contains(val1));
Assert.True(c.Contains(val1DifferentReference));
Assert.False(c.Contains(val2));

// Single item removal
Assert.True(c.Remove(val1));
Assert.Equal(0, c.Count);
Assert.False(c.Contains(val1));

// Multi-value
c.Add(val1);
c.Add(val2);
Assert.Equal(2, c.Count);
Assert.True(c.Contains(val1));
Assert.True(c.Contains(val1DifferentReference));
Assert.True(c.Contains(val2));
Assert.True(c.Contains(val1DifferentReference));
Assert.False(c.Contains(val3));

// Removal when multiple exist, using different reference.
Assert.True(c.Remove(val1));
Assert.False(c.Contains(val1));
Assert.True(c.Contains(val2));
Assert.Equal(1, c.Count);

// Removal of non-existent
Assert.False(c.Remove(val3));
Assert.False(c.Remove(val1DifferentReference));
Assert.Equal(1, c.Count);
Assert.True(c.Contains(val2DifferentReference));

// Removal last item
Assert.True(c.Remove(val2DifferentReference));
Assert.Equal(0, c.Count);
Assert.False(c.Contains(val2));
Assert.False(c.Contains(val1));

// Remove from empty
Assert.False(c.Remove(val1));
Assert.False(c.Remove(val2));
Assert.False(c.Remove(val3));
Assert.Equal(0, c.Count);
}
}
}