0

I recently came across this code:

    public static class ClientBaseExtender 
{ 
    /// <summary> 
    /// Tries to execute async service call. If <see cref="TimeoutException"/> occured retries again. 
    /// </summary> 
    /// <typeparam name="TChannel">ServiceClient class.</typeparam> 
    /// <typeparam name="TArgs">Type of service client method return argument.</typeparam> 
    /// <param name="client">ServiceClient instance.</param> 
    /// <param name="tryExecute">Delegate that execute starting of service call.</param> 
    /// <param name="onCompletedSubcribe">Delegate that subcribes an event handler to the OnCompleted event of the service client method.</param> 
    /// <param name="onCompleted">Delegate that executes when service call is succeeded.</param> 
    /// <param name="onError">Delegate that executes when service call fails.</param> 
    /// <param name="maxAttempts">Maximum attempts to execute service call before error if <see cref="TimeoutException"/> occured (by default 5).</param> 
    public static void ExecuteAsyncRepeatedly<TChannel, TArgs>(this ClientBase<TChannel> client, Action tryExecute, 
                                                               Action<EventHandler<TArgs>> onCompletedSubcribe, EventHandler<TArgs> onCompleted, 
                                                               EventHandler<TArgs> onError, int maxAttempts) 
        where TChannel : class 
        where TArgs : AsyncCompletedEventArgs 
    { 
        int attempts = 0; 
        var serviceName = client.GetType().Name; 

        onCompletedSubcribe((s, e) => 
                                { 
                                    if (e.Error == null) // Everything is OK 
                                    { 
                                        if (onCompleted != null) 
                                            onCompleted(s, e); 

                                        ((ICommunicationObject)client).Close(); 
                                        Debug.WriteLine("[{1}] Service '{0}' closed.", serviceName, DateTime.Now); 
                                    } 
                                    else if (e.Error is TimeoutException) 
                                    { 
                                        attempts++; 

                                        if (attempts >= maxAttempts) // Final timeout after n attempts 
                                        { 
                                            Debug.WriteLine("[{2}], Final Timeout occured in '{0}' service after {1} attempts.", serviceName, attempts, DateTime.Now); 

                                            if (onError != null) 
                                                onError(s, e); 
                                            client.Abort(); 

                                            Debug.WriteLine("[{1}] Service '{0}' aborted.", serviceName, DateTime.Now); 
                                            return; 
                                        } 

                                        // Local timeout 
                                        Debug.WriteLine("[{2}] Timeout occured in '{0}' service (attempt #{1}).", serviceName, attempts, DateTime.Now); 

                                        Debug.WriteLine("[{2}] Attempt #{0} to execute call to '{1}' service.", attempts + 1, serviceName, DateTime.Now); 
                                        tryExecute(); // Try again. 
                                    } 
                                    else 
                                    { 
                                        if (onError != null) 
                                            onError(s, e); 
                                        client.Abort(); 
                                        Debug.WriteLine("[{1}] Service '{0}' aborted.", serviceName, DateTime.Now); 
                                    } 
                                }); 

        Debug.WriteLine("[{2}] Attempt #{0} to execute call to '{1}' service.", attempts + 1, serviceName, DateTime.Now); 
        tryExecute(); // First attempt to execute 
    } 
}

    public void GetData()
    {
    var client = new MyServiceClient(); 
     client.ExecuteAsyncRepeatedly(() => client.MyOperationAsync(...), 
    (EventHandler<MyOperationCompletedEventArgs> handler)                                        =>client.MyOperationCompleted += handler, 
    (s, e) => // OnCompleted 
        { 
            Do(e.Result); 
        }, 
    (s, e) => // OnError 
        { 
            HandleError(e.Error); 
        } 
); 

}

The problem is, I have a button that fires this code off. When the button is pushed more than once the handler gets added again and again. This is a problem because the code will fire as many times as the user has pushed the button. How can I remove the handler created with the lambda expression in this code so it will only run once?

Thanks!

EDIT:

I'm calling the code like this from my button click command:

            _dataService.GetData(GetDataCompleted);

        private void GetDataComplete(Data data)
    {
        //do something with data        }
4
  • I'd suggest fixing that code block, half of it is not marked as code. Use the 010101 icon after highlighting your code block.
    – myermian
    Nov 19, 2010 at 21:16
  • Can you add in the actual code where the button click handler is? There is not enough here to determine why that handler is getting added many times. Nov 19, 2010 at 21:36
  • I don't know what you mean, it looks like the code is showing up properly within the code blocks to me.
    – adminJaxon
    Nov 20, 2010 at 17:37
  • steve, please see my useage of the code in my edits above. thanks for your help!
    – adminJaxon
    Nov 20, 2010 at 17:45

1 Answer 1

0

I think that you can solve it by implementing a push-pull strategy in your code-behind. I propose something similar to this:

 bool _requestPending;
 readonly object _lock = new object();

 void OnClick (...)
 {
     lock(_lock)
     {
        if (_requestPending == false)
        {
            _dataService.GetData(GetDataCompleted);
            _requestPending = true;
        }
     }
 }
 private void GetDataComplete(Data data)
 {
     lock(_lock)
     {
        try
        {
           //do something with data 
        }
        finally
        {
           _requestPending = false;
        }  
     }           
 }

Even better, disable the UI button when you have a pending request. You wouldn't have any concurrency issues with accessing and modifying the _requestPending from different threads, but still you could suffer a race condition if the service response is fast enough, so still better to synchronize the two blocks of code.

Anyway, personally I don't like this implementation for what you're trying to achieve. The code is quite confusing, and makes it hard to foresee problems that may arise. Make sure that:

  • you provide a way to abort a request and reenable the button again

  • the code that updates the screen is executed by the UI thread

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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