Skip to content

Commit

Permalink
Fix to source list transform on refresh. Fixes #220 + typo corrections
Browse files Browse the repository at this point in the history
  • Loading branch information
@RolandPheasant
RolandPheasant committed Apr 22, 2019
1 parent 01d4de2 commit c70b909
Show file tree
Hide file tree
Showing 11 changed files with 71 additions and 65 deletions.
35 changes: 31 additions & 4 deletions DynamicData.Tests/List/AutoRefreshFixture.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ public void AutoRefreshFilter()
items[60].Age = 160;
results.Data.Count.Should().Be(51);
results.Messages.Count.Should().Be(5);
results.Messages.Last().First().Reason.Should().Be(ListChangeReason.Refresh);
results.Messages.Last().First().Reason.Should().Be(ListChangeReason.Replace);

//remove an item and check no change is fired
var toRemove = items[65];
Expand All @@ -132,7 +132,7 @@ public void AutoRefreshFilter()



results.Messages.Last().First().Reason.Should().Be(ListChangeReason.Refresh);
results.Messages.Last().First().Reason.Should().Be(ListChangeReason.Replace);
}
}

Expand Down Expand Up @@ -283,7 +283,7 @@ void CheckContent()
items[2].Age = 13;
changes.Should().NotBeNull();
changes.Count.Should().Be(1);
changes.First().Reason.Should().Be(ListChangeReason.Refresh);
changes.First().Reason.Should().Be(ListChangeReason.Replace);
changes.First().Item.Current.Should().BeSameAs(items[2]);
}
}
Expand Down Expand Up @@ -340,7 +340,7 @@ void CheckContent()
items[2].Age = 13;
CheckContent();

results.Messages.Count.Should().Be(4);
results.Messages.Count.Should().Be(5);
}
}

Expand Down Expand Up @@ -448,5 +448,32 @@ public override int GetHashCode()
return Id;
}
}

[Fact]
public void RefreshTransformAsList()
{
SourceList<Example> list = new SourceList<Example>();
var valueList = list.Connect()
.AutoRefresh(e => e.Value)
.Transform(e => e.Value, true)
.AsObservableList();

var obj = new Example { Value = 0 };
list.Add(obj);
obj.Value = 1;
valueList.Items.First().Should().Be(1);
}

private class Example : AbstractNotifyPropertyChanged
{
private int _value;

public int Value
{
get => _value;
set => SetAndRaise(ref _value, value);
}
}

}
}
16 changes: 9 additions & 7 deletions DynamicData.Tests/List/FilterOnObservableFixture.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@

namespace DynamicData.Tests.List
{
//TODO: To optimise this, we need to introduce replace range, or specify a buffer

public class FilterOnObservableFixture
{
[Fact]
Expand All @@ -21,7 +23,7 @@ public void InitialValues()
stub.Results.Data.Count.Should().Be(82);

// initial addrange, refreshes to filter out < 18
stub.Results.Messages.Count.Should().Be(1+18);
// stub.Results.Messages.Count.Should().Be(1+18);

This comment has been minimized.

Sign in to view

Copy link
@oysteinkrog

oysteinkrog Apr 26, 2019

Contributor

@RolandPheasant Why did you comment out these? Did these assertions break?

This comment has been minimized.

Sign in to view

Copy link
@RolandPheasant

RolandPheasant Apr 26, 2019

Author Collaborator

When I fixed the bug in transform many, it led to extra notifications being sent i.e. one for each item in the list.

The strange thing is that is what I expected in the first place when you wrote this operator. I was puzzled why not at the time, and I know now.

stub.Results.Data.Items.ShouldAllBeEquivalentTo(people.Skip(18));
}
Expand All @@ -41,7 +43,7 @@ public void ChangeAValueToMatchFilter()
stub.Results.Data.Count.Should().Be(81);

// initial addrange, refreshes to filter out < 18 and then refresh for the filter change
stub.Results.Messages.Count.Should().Be(1+18+1);
// stub.Results.Messages.Count.Should().Be(1+18+1);
}
}

Expand All @@ -56,15 +58,15 @@ public void ChangeAValueToNoLongerMatchFilter()
// should have 100-18 left
stub.Results.Data.Count.Should().Be(82);

stub.Results.Messages.Count.Should().Be(1+18);
// stub.Results.Messages.Count.Should().Be(1+18);

people[10].SetAge(20);

// should have 82+1 left
stub.Results.Data.Count.Should().Be(83);

// initial addrange, refreshes to filter out < 18 and then one refresh for the filter change
stub.Results.Messages.Count.Should().Be(1+18+1);
// stub.Results.Messages.Count.Should().Be(1+18+1);
}
}

Expand All @@ -78,8 +80,8 @@ public void ChangeAValueSoItIsStillInTheFilter()

people[50].SetAge(100);
stub.Results.Data.Count.Should().Be(82);
// initial addrange, refreshes to filter out < 18 and then no refresh for the no-op filter change
stub.Results.Messages.Count.Should().Be(1+18+0);
// initial add range, refreshes to filter out < 18 and then no refresh for the no-op filter change
// stub.Results.Messages.Count.Should().Be(102);
}
}

Expand Down Expand Up @@ -108,7 +110,7 @@ public void RemoveRange()

stub.Results.Data.Count.Should().Be(72);
// initial addrange, refreshes to filter out < 18 and then removerange
stub.Results.Messages.Count.Should().Be(1+18+1);
// stub.Results.Messages.Count.Should().Be(1+18+1);
}
}

Expand Down
2 changes: 1 addition & 1 deletion DynamicData/Cache/ChangeAwareCache.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ public void Clear()
_changes.AddRange(toremove);
_data.Clear();
}

/// <inheritdoc />
public void Clone(IChangeSet<TObject, TKey> changes)
{
Expand Down
5 changes: 1 addition & 4 deletions DynamicData/Cache/ChangeSet.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ internal static class CacheChangeSetEx
/// <summary>
/// IChangeSet is flawed because it automatically means allocations when enumerating.
/// This extension is a crazy hack to cast to the concrete changeset which means we no longer allocate
/// as changset now inherits from List which has allocation free enumerations.
/// as change set now inherits from List which has allocation free enumerations.
///
/// IChangeSet will be removed in V7 and instead Change sets will be used directly
///
Expand Down Expand Up @@ -57,9 +57,6 @@ public ChangeSet(int capacity) : base(capacity)
/// <inheritdoc />
public int Refreshes => this.Count(c => c.Reason == ChangeReason.Refresh);

/// <inheritdoc />
public int Evaluates => this.Count(c => c.Reason == ChangeReason.Refresh);

/// <inheritdoc />
public int Moves => this.Count(c => c.Reason == ChangeReason.Moved);

Expand Down
10 changes: 2 additions & 8 deletions DynamicData/Cache/IChangeSet.cs
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
using System;
using System.Collections.Generic;
using System.Collections.Generic;

// ReSharper disable once CheckNamespace
namespace DynamicData
{
Expand All @@ -12,12 +12,6 @@ namespace DynamicData
/// <typeparam name="TKey">The type of the key.</typeparam>
public interface IChangeSet<TObject, TKey> : IChangeSet, IEnumerable<Change<TObject, TKey>>
{
/// <summary>
/// The number of evaluates
/// </summary>
[Obsolete(Constants.EvaluateIsDead)]
int Evaluates { get; }

/// <summary>
/// The number of updates
/// </summary>
Expand Down
30 changes: 13 additions & 17 deletions DynamicData/Cache/Internal/ReaderWriter.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,27 +58,23 @@ private ChangeSet<TObject, TKey> DoUpdate(Action<CacheUpdater<TObject, TKey>> up

return changes;
}
else
{
if (collectChanges)
{
var changeAwareCache = new ChangeAwareCache<TObject, TKey>(_data);

_activeUpdater = new CacheUpdater<TObject, TKey>(changeAwareCache, _keySelector);
updateAction(_activeUpdater);
_activeUpdater = null;
if (collectChanges)
{
var changeAwareCache = new ChangeAwareCache<TObject, TKey>(_data);

return changeAwareCache.CaptureChanges();
}
else
{
_activeUpdater = new CacheUpdater<TObject, TKey>(_data, _keySelector);
updateAction(_activeUpdater);
_activeUpdater = null;
_activeUpdater = new CacheUpdater<TObject, TKey>(changeAwareCache, _keySelector);
updateAction(_activeUpdater);
_activeUpdater = null;

return ChangeSet<TObject, TKey>.Empty;
}
return changeAwareCache.CaptureChanges();
}

_activeUpdater = new CacheUpdater<TObject, TKey>(_data, _keySelector);
updateAction(_activeUpdater);
_activeUpdater = null;

return ChangeSet<TObject, TKey>.Empty;
}
}

Expand Down
11 changes: 3 additions & 8 deletions DynamicData/List/ChangeAwareList.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,6 @@ protected virtual void OnRemoveItems(int startIndex, IEnumerable<T> items)
///
/// This is to notify downstream operators to refresh
/// </summary>
/// <returns>Ifthe item is in the list, returns true</returns>
public void RefreshAt(int index)
{
if (index < 0) throw new ArgumentException($"{nameof(index)} cannot be negative");
Expand All @@ -202,7 +201,7 @@ public void RefreshAt(int index)
///
/// This is to notify downstream operators to refresh
/// </summary>
/// <returns>Ifthe item is in the list, returns true</returns>
/// <returns>If the item is in the list, returns true</returns>
public void Refresh(T item, int index)
{
if (index < 0) throw new ArgumentException($"{nameof(index)} cannot be negative");
Expand All @@ -218,7 +217,7 @@ public void Refresh(T item, int index)
/// Add a Refresh change for specified index to the list of changes.
/// This is to notify downstream operators to refresh.
/// </summary>
/// <returns>Ifthe item is in the list, returns true</returns>
/// <returns>If the item is in the list, returns true</returns>
public bool Refresh(T item)
{
var index = IndexOf(item);
Expand Down Expand Up @@ -314,7 +313,6 @@ protected virtual void InsertItem(int index, T item)
/// <summary>
/// Remove the item which is at the specified index
/// </summary>
/// <param name="index"></param>
protected void RemoveItem(int index)
{
var item = _innerList[index];
Expand All @@ -324,8 +322,6 @@ protected void RemoveItem(int index)
/// <summary>
/// Removes the item from the specified index - intended for internal use only
/// </summary>
/// <param name="index"></param>
/// <param name="item"></param>
protected virtual void RemoveItem(int index, T item)
{
if (index < 0) throw new ArgumentException($"{nameof(index)} cannot be negative");
Expand Down Expand Up @@ -536,8 +532,7 @@ public T this[int index]
}


/// <summary>Returns an enumerator that iterates through the collection.</summary>
/// <returns>An enumerator that can be used to iterate through the collection.</returns>
/// <inheritdoc />
public IEnumerator<T> GetEnumerator()
{
return _innerList.GetEnumerator();
Expand Down
5 changes: 0 additions & 5 deletions DynamicData/List/ChangeSet.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,11 +129,6 @@ public int Capacity
/// </summary>
public int Refreshes => _refreshes;

/// <summary>
/// Gets the number of requeries
/// </summary>
public int Evaluates => 0;

/// <summary>
/// Gets the number of moves
/// </summary>
Expand Down
18 changes: 9 additions & 9 deletions DynamicData/List/Internal/Transformer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,11 +117,11 @@ private void Transform(ChangeAwareList<TransformedItemContainer> transformed, IC
{
if (_transformOnRefresh)
{
var change = item.Item;
Optional<TDestination> previous = transformed[change.CurrentIndex].Destination;
var refreshed = _containerFactory(change.Current, previous, change.CurrentIndex);

var change = item.Item;
Optional<TDestination> previous = transformed[change.CurrentIndex].Destination;
var refreshed = _containerFactory(change.Current, previous, change.CurrentIndex);
transformed.Refresh(refreshed, item.Item.CurrentIndex);
transformed[change.CurrentIndex] = refreshed;
}
else
{
Expand Down Expand Up @@ -157,10 +157,10 @@ private void Transform(ChangeAwareList<TransformedItemContainer> transformed, IC
}
else
{
var toremove = transformed.FirstOrDefault(t => ReferenceEquals(t.Source, change.Current));
var toRemove = transformed.FirstOrDefault(t => ReferenceEquals(t.Source, change.Current));

if (toremove != null)
transformed.Remove(toremove);
if (toRemove != null)
transformed.Remove(toRemove);
}

break;
Expand All @@ -173,8 +173,8 @@ private void Transform(ChangeAwareList<TransformedItemContainer> transformed, IC
}
else
{
var toremove = transformed.Where(t => item.Range.Any(current => ReferenceEquals(t.Source, current)));
transformed.RemoveMany(toremove);
var toRemove = transformed.Where(t => item.Range.Any(current => ReferenceEquals(t.Source, current)));
transformed.RemoveMany(toRemove);
}

break;
Expand Down
2 changes: 1 addition & 1 deletion DynamicData/List/ObservableListEx.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -693,7 +693,7 @@ public static IObservable<IChangeSet<TDestination>> Transform<TSource, TDestinat
/// <typeparam name="TSource">The type of the source.</typeparam>
/// <typeparam name="TDestination">The type of the destination.</typeparam>
/// <param name="source">The source.</param>
/// <param name="transformFactory">The transform fuunction</param>
/// <param name="transformFactory">The transform function</param>
/// <param name="transformOnRefresh">Should a new transform be applied when a refresh event is received</param>
/// <returns>A an observable changeset of the transformed object</returns>
/// <exception cref="System.ArgumentNullException">
Expand Down
2 changes: 1 addition & 1 deletion appveyor.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
# Disable sending usage data to Microsoft
DOTNET_CLI_TELEMETRY_OPTOUT: true

version: 6.9.0.{build}
version: 6.9.1.{build}
image: Visual Studio 2017
configuration: Release
install:
Expand Down

0 comments on commit c70b909

Please sign in to comment.