6
\$\begingroup\$

Use case

A GUI with 2 controls:

  • An input control for text (a DNA sequence) that should be analyzed
  • A result control that displays the analyzing result of the entered text

The analysis takes 2-3 seconds and should start as soon as the text in the input control is changing. The shown result should always match the actual text in the input field.

Problem

If the user types text, it changes multiple times within the processing time of the analysis. Starting multiple processing tasks parallel is not an option.

Solution

My approach for solving the problem above:

  • An 'ItemProcessor' gets the input and process it in background.
  • If a new item was added whereas another item is processing, the new item is 'stored' for subsequent processing.
  • If another item is already stored for subsequent processing, it will be overwritten.
  • The result is returned only, of no item for subsequent processing is available anymore.

Implementation:

public class LastItemProcessor<TInput, TOutput>
{
    private readonly Func<TInput, Task<TOutput>> Process;
    private readonly Action OnProcessingStartedAction;
    private readonly Action OnProcessingFinishedAction;
    private readonly Action<Task<TOutput>> OnLastItemProcessedAction;
    private TInput myInput;
    private bool myIsLastItemFlag = true;
    private bool myIsProcessing = false;
    private readonly object myLock = new object();

    public LastItemProcessor(Func<TInput, Task<TOutput>> process,
        Action<Task<TOutput>> onLastItemProcessedAction,
        Action onProcessingStartedAction = null,
        Action onProcessingFinishedAction = null)
    {
        process.Ensure(nameof(process)).IsNotNull();
        onLastItemProcessedAction.Ensure(nameof(onLastItemProcessedAction)).IsNotNull();

        Process = process;
        OnProcessingStartedAction = onProcessingStartedAction;
        OnProcessingFinishedAction = onProcessingFinishedAction;
        OnLastItemProcessedAction = onLastItemProcessedAction;
    }

    public void ProcessItem(TInput input)
    {
        lock (myLock)
        {
            myInput = input;

            if (myIsProcessing)
            {
                myIsLastItemFlag = false;
            }
            else
            {
                myIsProcessing = true;
                OnProcessingStartedAction?.Invoke();
                var task = ProcessNextItemAsync();
                task.ContinueWith(t =>
                {
                    OnLastItemProcessedAction(task);
                    OnProcessingFinishedAction?.Invoke();
                });
            }
        }
    }

    private async Task<TOutput> ProcessNextItemAsync()
    {
        TOutput result;
        try
        {
            result = await Process(myInput);
        }
        catch (Exception)
        {
            // ensure that the processing flag is set in case of exceptions.
            myIsProcessing = false;
            throw;
        }
        lock (myLock)
        {
            if (myIsLastItemFlag)
            {
                myIsProcessing = false;
                return result;
            }
            myIsLastItemFlag = true;
        }
        return await ProcessNextItemAsync();
    }
}

Usage

// Initialization (in constructor)
// Note: AnalyzingError, IsAnalyzing and AnalysisRegions are view model properties
myLastItemProcessor = new LastItemProcessor<string, AnalysisRegion[]>(
            AnalyzeSequenceAsync,
            t =>
            {
                if (t.IsFaulted)
                {
                    AnalyzingError = "Error Message";
                }
                else
                {
                    AnalysisRegions = t.Result;
                    AnalyzingError = null;
                }
            },
            () => IsAnalyzing = true,
            () => IsAnalyzing = false);

// ...
// Process item (in setter of Sequence property)
myLastItemProcessor.ProcessItem(sequence);

I am interested in general code improvements (e.g. naming, simplifications, bugs, ...) and alternative approaches to address that kind of problem.

However, any constructive feedback is welcome of course :)

\$\endgroup\$
  • 1
    \$\begingroup\$ This time I won't write it in the review but I believe you know your favourite member prefix will be mentioned (again) :-P \$\endgroup\$ – t3chb0t May 12 '17 at 19:48
  • \$\begingroup\$ Probably... Actually, I've been thinking about changing it to '_' for a brief moment. However I like too much, so I'll use it in all my questions until it becomes the standard of the Microsoft coding conventions :P \$\endgroup\$ – JanDotNet May 14 '17 at 11:45
2
\$\begingroup\$

There are two alternatives:

  1. Reactive extensions with Throttle + Switch combo. It will take care of throttling text changes, overriding previous results and scheduling tasks. Rx API is not for the faint-hearted though. See: https://stackoverflow.com/a/22924151/1386995 .
  2. TPL Dataflow with BroadcastBlock + ActionBlock combo. It will take care about everything except for throttling. You will have to initialize those blocks with correct buffer sizes and degrees of parallelism.

Also note, that Dataflow blocks can be converted to Rx observables with extension methods, so you can combine those frameworks.


As for your implementation:

  1. if

    The shown result should always match the actual text in the input field.

    Then you should probably cancel incomplete processing when input changes. If you are going to discard the result anyway, then there is no reason to wait for completion. 3s is a lot of time.

  2. I don't have a clear understanding on how recursion works with async/await. It can behave differently. Maybe you should take extra precautions to make sure that stack can not overflow.

  3. I'm not a fan of my prefixes. But that's a personal preference.

  4. myIsLastItemFlag name is a bit confusing. I think something like myIsInputChanged would do a better job at explaining what this flag is for. Also Flag postfix is unnecessary, Is prefix already hints, that it is a bool field.

\$\endgroup\$
  • \$\begingroup\$ 1. Unfortunately, it is not possible to cancel the process, because the calculation is done by another application (lets call it ProgX), started as separate process. ProgX produces temporary files therefore it is not practicable to kill the process. On the other hand, just canceling the task may result in lots for ProgX processes... So I am using the execution time of ProgX as delay. \$\endgroup\$ – JanDotNet Jun 7 '17 at 8:10
  • \$\begingroup\$ 2. Because it is a tail recursion, I thought that compiler translates it to a loop. But I'll check it. \$\endgroup\$ – JanDotNet Jun 7 '17 at 8:13
  • \$\begingroup\$ 4. You are right, myIsInputChanged sounds better. \$\endgroup\$ – JanDotNet Jun 7 '17 at 8:14
  • \$\begingroup\$ Thanks for the hint with the TPL Dataflow. It look very promises, so I'll give it a shot. But I didn't figured out hot to realize my requirement with exiting blocks. The WriteOnceBlock takes the first input and ignores subsequent ones. But I need a block that caches only the last input and forgets previous ones. Is that possible without writing my own block implementation? \$\endgroup\$ – JanDotNet Jun 7 '17 at 8:22
  • 1
    \$\begingroup\$ The best solution I came up with is a BufferBlock with BoundedCapacity of 1 and calling buffer.TryReceive(out item); before buffer.Post(newItem);. Anyway.. It's a really intresting library so thanks for recommendation :). \$\endgroup\$ – JanDotNet Jun 7 '17 at 9:10

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.