2

I need to start a download of some html so I call void GetHTML in another class. When it's done I want to pass along what Event it should raise in the calling class. How could I do this?

So it would look something like this:

public class Stuff
{
    public void GetHTML(string url, event to raise here)
    {
       //Do stuff then raise event
    }
}
public class Other
{
    public Other()
    {
        Stuff stuff = new Stuff();
        stuff.GetHTML(someUrl, somehow sending info that HTML_Done should be called);
    }
    void HTML_Done(string result, Event e)
    {
         //Do stuff with the result since it's done
    }
}

I realize I'm not super clear with what I want to do, I'd be glad to fill in any missing parts.

Thanks for any suggestions!

Phil
  • 3,934
  • 12
  • 38
  • 62

5 Answers5

12

Event Subscription and Notification

public class Stuff
{
    // Public Event to allow other classes to subscribe to.
    public event EventHandler GetHtmlDone = delegate { };

    public void GetHTML(string url)
    {
        //Do stuff

        // Raise Event, which triggers all method subscribed to it!
        this.GetHtmlDone(this, new EventArgs());
    }
}

public class Other
{
    public Other()
    {
        Stuff stuff = new Stuff();

        // Subscribe to the event.
        stuff.GetHtmlDone += new EventHandler(OnGetHtmlDone);

        // Execute
        stuff.GetHTML("someUrl");
    }

    void OnGetHtmlDone(object sender, EventArgs e)
    {
        //Do stuff with the result since it's done
    }
}

Using this pattern allows many more subscribers.

You also do not tie the notifier, the Stuff class, to the caller, the Other class.

You either have subscribers or not, no difference to the Stuff class.

The Stuff class should not know about the subscriber, it should merely raise an event it exposes for subscription.

EDIT
As ctacke pointed out correctly in the comments, raising the event using this.GetHtmlDone(this, new EventArgs()); will cause an exception if nobody has subscribed.
I changed my code above to ensure the event can be raised safely at all times by initialising my eventhandler.
As I'm always using it (by raising it) I'm sure it is only good practise to always initialise what you are using.

I could have added a null check on the event handler but in my personal oppinion I do not agree with that having to be the responsibility of the stuffclass. I feel the event should always be raised as it is the "responsible" thing to do.

I found this thread on SO which kind of confirmed to me that it does not seem wrong to do so.

In addition I also run Code Analysis on that code to ensure I'm not breaking the CA1805 rule by initialising the EventHandler. CA1805 was not raised and no rules have been broken.

Using my car analogy from the comments, I believe not initializing the eventhandler and raising it all the time would be the same than saying "When turning a corner in your car only use your indicator if someone is watching and if not don't bother". You never know if anyone is watching, so you might as well make sure you always do it.

This is simply my personal preference. Anyone else, please do add the != null check at all times if that is how you prefere to do it.

Thank you so much for the comments ctacke and for pointing this out. I learned quite a lot from this.
I will have to go back to some of my projects now and update some code to make sure my libraries are not crashing if nobody subscribes to my events. I feel quite silly not ever having caught that in any of my Tests.

Community
  • 1
  • 1
Nope
  • 22,147
  • 7
  • 47
  • 72
  • You can even make the EventArg a custom EventArg passing important information to the subscriber. – Nope Jun 01 '11 at 13:07
  • 1
    Though you should only call `GetHtmlDone` if it has a subscriber – ctacke Jun 01 '11 at 13:23
  • I raise the GetHtmlDone event regardless of subscriptions. The `Stuff`class is not reponsible for checking for subscriptions. The `Stuff`class has no dependency on the subscribers. The subscribers have a dependency on the event being raised. Using an analogy, I see it like using my indicator in my car when turning a corner. I always do it, even if no one is watching (subscribed). I have no dependency on the car behind me watching me use my indicator, however, the car behind me has a dependency on me indicating (raising my event). At least that is how I see it. – Nope Jun 01 '11 at 18:34
  • Sure, `Other` adds a handler, so what you have now will work, but if the code is refactored or shared and someone else uses `Stuff` but doesn't wire up the GetHtmlDone, then the call to this.GetHtmlDone is going to throw a NullReferenceException. Don't code for the problem at hand, code for what someone other than you might do when you're not looking. – ctacke Jun 01 '11 at 21:46
  • Not sure what you mean cause `stuff`will never throw an exception. Maybe I'm missing something but `stuff`can be referenced by anything with or without subscription. Sorry if I'm missing something. I'm quite interested in the "how" of what you are saying because I'm all for preventing exceptions. – Nope Jun 01 '11 at 22:47
  • Simple. Using your `Stuff` class above, try this in a test or empty project: [TestMethod()] public void StuffTest() { var stuff = new Stuff(); stuff.GetHTML(string.Empty); } – ctacke Jun 01 '11 at 23:24
  • OK, now, I tried it and yes, I got an exception. Next I thought that I don;t like the fact I should be checking for null as it should not be the `stuff` classes reponsibility. that is my personal oppinion. To use my car indicator analogy it implies you should only use your indicator if someone is watching, otherwise don't. To that end I have found that you can initialise the eventhandeler with an empty delegate and avoid the null check. In addition I run code analysis on it and it does not have an issue with it either as we are initialising a variable which we are using, by raising the event. – Nope Jun 02 '11 at 09:01
  • I updated my solution, with additonal references to reflect the changes. – Nope Jun 02 '11 at 09:02
  • This answer is just so good I had to comment even more than a year later. – Phil Aug 13 '12 at 15:28
5

I'm not sure if this is what you're after, but you can use the built in Action delegate type:

public class Stuff
{
  public void GetHTML(string url, Action<string, Event> callback)
  {
     //Do stuff 

     //raise event
     callback("result here", new Event());       
  }
}

stuff.GetHTML(someUrl, HTML_Done);

Alternatively, use the standard event pattern and the EventHandler<T> delegate type. You'll need to create your own EventArgs type.

devdigital
  • 34,151
  • 9
  • 98
  • 120
  • Why are you using an `Action`? A simple `Action` is more clear and sufficient. – Zebi Jun 01 '11 at 12:59
  • 1
    The OP's HTML_Done signature includes an Event type, which I assume he wants included. The assumption is based on the equivocality of the question. – devdigital Jun 01 '11 at 13:06
  • @dev, I'm assuming he included the Event in the signature because he assumed an Event was required. – Josh Smeaton Jun 01 '11 at 13:10
  • Yes I also thought of that possibility, but I chose not to assume ignorance on their part. Which was ignorant of me. – devdigital Jun 01 '11 at 13:11
  • I was more upset that they have an underscore in their method name, and no explicit access modifier. – devdigital Jun 01 '11 at 13:13
  • What if the class calling GetHtml is not interested in being notified? Why force it to pass an action object? Let the `Stuff` class raise an event. Expose it through a public event handler and any class interested in a notification can subscribe to it. That way you give the option of being notified instead of forcing it upon the calling object. I'm assuming that is what the alternative option was hinting at. – Nope Jun 01 '11 at 13:20
  • Not ignorant at all, cautious maybe :P and yes, the underscore got to me too! – Josh Smeaton Jun 01 '11 at 13:20
  • @Fran, or make the callback argument optional? I much prefer callbacks to events, especially when you're working with async code. Chances are going to be quite high that you're interested in the result. I consider events useful for "I'm doing something you might be interested in", not "you asked for something and I'll shout it to the world". – Josh Smeaton Jun 01 '11 at 13:24
  • @François Yes good point, I mention the standard event pattern, you could also make the callback discretionary using optional arguments in C# 4.0, or method overloading in prior versions – devdigital Jun 01 '11 at 13:26
1

You want a callback.

public class Stuff
{
    public void GetHTML(string url, Action callback)
    {
       // Do stuff

       // signal we're done 
       callback();
    }
}

public class Other
{
    public Other()
    {
        Stuff stuff = new Stuff();

        // using a lambda callback
        stuff.GetHTML(someUrl, ()=> Console.WriteLine("Done!") );
        // passing a function directly
        stuff.GetHTML(someUrl, MyCallback);
    }

    public void MyCallback() 
    {
        Console.WriteLine("Done!");
    }
}

If you want to pass arguments, define the Action appropriately, ie; Action<string>, and then the callback becomes callback("some string")

The other option you have, is by using Events. It looks like that was what you were aiming for when you asked the question. I wouldn't recommend it, the callback option is nicer IMO. Though, for future reference, this is how you'd use an Event in this circumstance.

public delegate void DoneEventHandler(object sender, string result);

public class Stuff
{
    public event DoneEventHandler DoneEvent = delegate {}; // avoid null check later

    public void GetHtml(string url)
    {
        // do stuff
        DoneEvent(this, "result");
    }
}

public class Other
{
    public void SomeMethod()
    {
        Stuff stuff = new Stuff();
        stuff.DoneEvent += OnDone;
        stuff.GetHtml(someUrl)
    }

    public void OnDone(objects sender, string result)
    {
        Console.WriteLine(result);
    }
}

Info on how to use Events in C#.

Josh Smeaton
  • 47,939
  • 24
  • 129
  • 164
0

If I ve not understood you wrong.I think you should try this way

public class Stuff 
{
     public void GetHTML(string url, event to raise here)
     {
        //Do stuff  and instead of raising event here
     }
 } 
public class Other
 {
     public Other()
     {
         Stuff stuff = new Stuff();
         stuff.GetHTML(someUrl, somehow sending info that HTML_Done should be called);
         //Raise event here once that method is finished
     }
     void HTML_Done(string result, Event e)
     {
          //Do stuff with the result since it's done
     }
 } 
Umesh CHILAKA
  • 1,466
  • 14
  • 25
0


You can try this approach


public class Stuff
{
     public delegate void DownloadComplete("you can pass whatever info you like here");
     public event DownloadComplete OnDownloadComplete;
     public void GetHtml(string UrlToDownload)
     {
         //Download the data here
         //After data download complete
         if(OnDownloadComplete)
            OnDownloadComplete("arguments to be passed");
     }

} public class Other { public Other() { Stuff myStuff=new Stuff(); myStuff.OnDownloadComplete+=new EventHandler(myStuff_OnDownloadComplete); }

 void myStuff_OnDownloadComplete("arguments will be passed here")
 {
    //Do your stuff
 }

}

Amresh Kumar
  • 1,445
  • 17
  • 30