Reputation: 161
I have a problem with some code I need to refactor. Right now it uses lambdas as event handlers, but they are not removed properly. From what I have read, this is not even possible? Anyway I would like to rewrite it to use a delegate instead of an anonymous function, and now my problem is that right now it takes an action as parameter, and I can't seem to figure out how to pass the action on to my new delegate. This is the code:
void RetrieveData(
int pointId,
int? chartCollectionId,
Action action)
{
if (pointId <= 0)
throw new ArgumentException("PointId not valid");
LastPointId = NextPointId;
NextPointId = pointId;
Clear();
_csr = new CustomerServiceRepository();
_csr.ServiceClient.GetChartDataCompleted += (se, ea) =>
{
_cachedCharts = ea.Result;
ChartDataRetrieved(ea.Result);
if (action != null)
action.Invoke();
_csr = null;
};
_csr.ServiceClient.GetChartDataAsync(
Settings.Current.Customer.CustomerName,
pointId,
chartCollectionId);
_csr.ServiceClient.GetChartDataCompleted -= (se, ea) => //remove after usage
{
_cachedCharts = ea.Result;
ChartDataRetrieved(ea.Result);
if (action != null)
action.Invoke();
_csr = null;
};
}
I was thinking that maybe I could create the following:
public class extendedEventArgs : GetChartDataCompletedEventArgs
{
Action foo { get; set; }
}
void tang(object sender, extendedEventArgs e)
{
_cachedCharts = e.Result;
ChartDataRetrieved(e.Result);
if (action != null)
action.Invoke();
_csr = null;
}
And the pass the action as a parameter in the extended event args, but when I try to use it like this
_csr.ServiceClient.GetChartDataCompleted += new EventHandler<extendedEventHandler>(tang);
It gives an error:
Cannot implicitly convert type System.EventHandler<Conwx.Net.Client.CustomerClient.Controls.ChartControls.ChartListForecast.extendedEventArgs>' to System.EventHandler<Conwx.Net.Client.Framework.CustomerServiceReference.GetChartDataCompletedEventArgs>'
What am I doing wrong here? Alternative solutions are also welcome.
.K
Upvotes: 0
Views: 4015
Reputation: 4110
If you'd prefer to avoid the anonymous methods, you can manually do essentially what the compiler is doing for you under the hood. That is, create a closure class to hold the Action and a reference to itself as fields and which exposes the method you want to assign to the event. Something like this:
class RetrieveDataClosure
{
private Action action;
private MyClass self;
public RetrieveDataClosure(Action action, MyClass self)
{
this.action = action;
this.self = self;
}
public void ChartDataCompleted(object se, MyEventArgs ea)
{
self._cachedCharts = ea.Result;
self.ChartDataRetrieved(ea.Result);
if (action != null)
action.Invoke();
self._csr = null;
}
}
Which you'd use in your code like this:
var closure = new RetrieveDataClosure(action, this);
_csr = new CustomerServiceRepository();
_csr.ServiceClient.GetChartDataCompleted += closure.ChartDataCompleted;
_csr.ServiceClient.GetChartDataAsync(
Settings.Current.Customer.CustomerName,
pointId,
chartCollectionId);
_csr.ServiceClient.GetChartDataCompleted -= closure.ChartDataCompleted;
Upvotes: 0
Reputation: 1499770
No, you can't do this because it's the class which raises the GetChartDataCompleted
event which creates the object passed (as a reference) to the event handler. It will be creating a GetChartDataCompletedEventArgs
- not an extendedEventArgs
.
If you think about it, it's like trying to implement an interface which looks like this:
public interface IFoo
{
void Foo(object x);
}
with a class like this:
public class Bar : IFoo
{
// We don't care if someone calling IFoo wants to pass us something
// other than a string - we want a string, darn it!
public void Foo(string y)
{
Console.WriteLine(y.Length);
}
}
That's clearly not going to work...
Marc has shown one approach to fixing it - but I'd also point out that you should probably actually only be removing the delegate when the event fires. I'm assuming that the fact that the method is called GetChartDataAsync
means it's a non-blocking method... so unsubscribing from the event immediately after calling it probably isn't a great idea.
Upvotes: 1
Reputation: 1062502
As I read it, the key problem here is not being able to remove the handler; if so, all you need it to store the delegate (where in the below, YourDelegateType
is meant to mean: the defined type of GetChartDataCompleted
):
YourDelegateType handler = (se, ea) =>
{
_cachedCharts = ea.Result;
ChartDataRetrieved(ea.Result);
if (action != null)
action.Invoke();
_csr = null;
};
_csr.ServiceClient.GetChartDataCompleted += handler;
...
_csr.ServiceClient.GetChartDataCompleted -= handler;
You can also make it self-unsubscribing (i.e. so that it unsubscribes when the event is raised):
YourDelegateType handler = null;
handler = (se, ea) =>
{
_cachedCharts = ea.Result;
ChartDataRetrieved(ea.Result);
if (action != null)
action.Invoke();
_csr.ServiceClient.GetChartDataCompleted -= handler;
_csr = null;
};
_csr.ServiceClient.GetChartDataCompleted += handler;
Upvotes: 2