Seeking feedback on TransformWithSuspendableInlineUpdates implementation

[Gabrielelrusso]

Hi guys!

I've implemented a TransformWithSuspendableInlineUpdates extension method. I'd appreciate any feedback or suggestions for improvement.

My use case: I'm mapping the elements of my source cache to view models, I want to suspend updates on a view model while the user is doing some operations with it. Updates are resumed (the latest change is applied) once the user is done.

    public static class ObservableChangesetExtensions
    {
        /// <summary>
        /// Transforms an observable change set with inline updates that can be suspended based on a condition.
        /// </summary>
        public static IObservable<IChangeSet<TDestination, TKey>> TransformWithSuspendableInlineUpdates<TSource, TDestination, TKey>(
            this IObservable<IChangeSet<TSource, TKey>> source,
            Func<TSource, TDestination> transformFactory,
            Action<TDestination, TSource> updateAction,
            Func<TDestination, IObservable<bool>> suspendUpdatesObservable)
            where TDestination : notnull
            where TKey : notnull
            where TSource : notnull
        {
            return source
                // Transforming source items to include their keys, used in WatchValue
                .Transform((item, key) => (Item: item, Key: key))
                .TransformWithInlineUpdate(keyedItem =>
                    {
                        // The first time we create the destination item,
                        // then the DestinationUpdater handles any update to the item
                        var destinationItem = transformFactory(keyedItem.Item);
                        return new DestinationUpdater<TSource, TDestination>(
                            destinationItem,
                            source.WatchValue(keyedItem.Key),
                            updateAction,
                            suspendUpdatesObservable);
                    },
                    // Ignore updates, as they are handled in the DestinationUpdater
                    (_, _) => { })
                .DisposeMany()
                .Transform(updater => updater.DestinationItem);
        }

        /// <summary>
        /// Updates the given destination item when the source item observable emits a new value.
        /// Updates are suspended so long the suspend updates observable holds true.
        /// </summary>
        private class DestinationUpdater<TSource, TDestination> : IDisposable
        {
            private readonly IDisposable _subscription;

            public DestinationUpdater(
                TDestination destinationItem,
                IObservable<TSource> sourceItemObservable,
                Action<TDestination, TSource> updateAction,
                Func<TDestination, IObservable<bool>> suspendUpdatesObservable)
            {
                // Combine source changes with suspension state
                _subscription = sourceItemObservable
                    .CombineLatest(suspendUpdatesObservable(destinationItem),
                        (source, areUpdatesSuspended) => (SourceItem: source, AreUpdatesSuspended: areUpdatesSuspended))
                    .Where(tuple => !tuple.AreUpdatesSuspended) // Only proceed when updates are not suspended
                    .Select(tuple => tuple.SourceItem)
                    .Subscribe(sourceItem => updateAction(destinationItem, sourceItem));

                DestinationItem = destinationItem;
            }

            public TDestination DestinationItem { get; }

            public void Dispose()
            {
                _subscription.Dispose();
            }
        }
    }

[JakenVeina]

Looks reasonable to me. Not how I would implement it, but that's just because I try and avoid making DynamicData operators by composing other DynamicData operators. I.E. creating dependencies where they don't need to be.

The repeated use of .Transform() is problematic, due to how heavy it is. That's a full complete copy of the collection for each operator call. You can swap them both to .TransformImmutable() in this case.

[Gabrielelrusso]

Thank you for the feedback.

That's a good point; composing operators is quite handy since it lets me build on top of well established and tested operators, but I'll keep your consideration in mind.

I'll swap the two Transform for TransformImmutable as you suggested. Thanks for the tip! I'll be more mindful about which of the two operators to use going forward.

Thanks again for maintaining the library and for your help!