Skip to content

Commit

Permalink
Call AddRange and InsertRange from corresponding Builder methods
Browse files Browse the repository at this point in the history
 - This fixes failing InsertRangeRandomBalanceTest
Also refactor AddRangeBalanceTest to add random batches and verify
  balance after each
  • Loading branch information
PatrickMcDonald committed Nov 16, 2014
1 parent da5f9d8 commit b516981
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -794,10 +794,7 @@ public void AddRange(IEnumerable<T> items)
{
Requires.NotNull(items, "items");

foreach (var item in items)
{
this.Root = this.Root.Add(item);
}
this.Root = this.Root.AddRange(items);
}

/// <summary>
Expand All @@ -817,10 +814,7 @@ public void InsertRange(int index, IEnumerable<T> items)
Requires.Range(index >= 0 && index <= this.Count, "index");
Requires.NotNull(items, "items");

foreach (T item in items)
{
this.Root = this.Root.Insert(index++, item);
}
this.Root = this.Root.InsertRange(index, items);
}

/// <summary>
Expand Down
35 changes: 28 additions & 7 deletions src/System.Collections.Immutable/tests/ImmutableListTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -161,22 +161,35 @@ public void AddRangeOptimizationsTest()
[Fact]
public void AddRangeBalanceTest()
{
int randSeed = (int)DateTime.Now.Ticks;
Console.WriteLine("Random seed: {0}", randSeed);
var random = new Random(randSeed);

int expectedTotalSize = 0;

var list = ImmutableList<int>.Empty;

// Add batches of 32, 128 times, giving 4096 items
int batchSize = 32;
// Add some small batches, verifying balance after each
for (int i = 0; i < 128; i++)
{
list = list.AddRange(Enumerable.Range(batchSize * i + 1, batchSize));
VerifyBalanced(list.Root);
int batchSize = random.Next(32);
Console.WriteLine("Adding {0} elements to the list", batchSize);
list = list.AddRange(Enumerable.Range(expectedTotalSize+1, batchSize));
VerifyBalanced(list);
expectedTotalSize += batchSize;
}

// Add a single large batch to the end
list = list.AddRange(Enumerable.Range(4097, 61440));
Assert.Equal(Enumerable.Range(1, 65536), list);
int largeBatchSize = random.Next(32768) + 32768;
Console.WriteLine("Adding {0} elements to the list", largeBatchSize);
list = list.AddRange(Enumerable.Range(expectedTotalSize + 1, largeBatchSize));
VerifyBalanced(list);
expectedTotalSize += largeBatchSize;

VerifyBalanced(list.Root);
Assert.Equal(Enumerable.Range(1, expectedTotalSize), list);

// The following is not guaranteed by AVL trees but has not failed
// for any iterations of this test so far
// Ensure that tree height is no more than 1 from optimal
var root = list.Root as IBinaryTree<int>;

Expand Down Expand Up @@ -214,13 +227,16 @@ public void InsertRangeRandomBalanceTest()

Assert.Equal(list, immutableList);

/* We don't actually have this guarantee for AVL trees
An example where the following assertion fails is with randSeed = -69928973;
// Ensure that tree height is no more than 1 from optimal
var root = immutableList.Root as IBinaryTree<int>;
var optimalHeight = Math.Ceiling(Math.Log(root.Count, 2));
Console.WriteLine("Tree depth is {0}, optimal is {1}", root.Height, optimalHeight);
Assert.InRange(root.Height, optimalHeight, optimalHeight + 1);
*/
}

[Fact]
Expand Down Expand Up @@ -736,6 +752,11 @@ internal override IImmutableListQueries<T> GetListQuery<T>(ImmutableList<T> list
return list;
}

private static void VerifyBalanced<T>(ImmutableList<T> tree)
{
VerifyBalanced(tree.Root);
}

private static void VerifyBalanced<T>(IBinaryTree<T> node)
{
if (node.Count <= 2)
Expand Down

0 comments on commit b516981

Please sign in to comment.