Dzisiaj prosta i częsta sprawa – obsługa zdarzeń. Zdefiniujmy fikcyjne zdarzenie:
class SampleClass { public event EventHandler EventHappened; }
Następnie mamy jakąś metodę, która po wykonaniu własnej logiki, wywołuje zdarzenie. Jak powinno to zostać prawidłowo wykonane? Zacznijmy od niepoprawnego podejścia:
class SampleClass { public event EventHandler EventHappened; private void Method() { EventHappened(this,new EventArgs()); } }
Metoda spowoduje wyjątek jeśli żadne zdarzenie nie zostało doczepione. Rozwiązaniem jest klasyczne podejście, zgodne z guidelines:
class SampleClass { public event EventHandler EventHappened; private void Method() { OnEventHappened(new EventArgs()); } virtual protected void OnEventHappened(EventArgs e) { if (EventHappened != null) EventHappened(this, e); } }
Stworzyliśmy więc wirtualną metodę protected zaczynająca się od przedrostka On, przyjmującą jako parametr wejściowy argumenty zdarzenia. W ciele metody sprawdzamy, czy są jakieś subskrypcje podpięte i następnie wywołujemy zdarzenie.
Rozwiązanie poprawne, zgodne z guidelines i większości przypadków właśnie w taki sposób obsługuję zdarzenia. Chciałbym jednak pokazać inne podejście, które przede wszystkim pomaga w dwóch kwestiach:
- Powyższa implementacja nie jest thread-safe. Jeśli dwa wątki niefortunnie wywołają OnEventHappened może dojść do problemu z synchronizacją warunku IF.
- Niektórzy programiści są bardzo przeczuleni na punkcie IF w programowaniu obiektowym. Zwykle możemy się ich pozbyć poprzez polimorfizm lub Null Object pattern.
Rozwiązaniem jest przypisanie po prostu domyślnej wartości:
public event EventHandler EventHappened = delegate { };
Całość:
class SampleClass { public event EventHandler EventHappened = delegate { }; private void Method() { OnEventHappened(new EventArgs()); } virtual protected void OnEventHappened(EventArgs e) { EventHappened(this, e); } }
Rozwiązanie polega na wykorzystaniu Null Object Pattern. Nie potrzebujemy już instrukcji IF – zawsze jest podpięte przynajmniej jedno zdarzenie.
Ja preferuje przypisanie handlera do zmiennej lokalnej, dzięki temu nie złapiemy się na null reference exception w przypadku wielu wątków.
Tworzenie domyślnej wartości ma swój narzut co w przypadku posiadania dużej ilości obiektów jest odczuwalne i ja go nie stosuję, bo 1 if krzywdy nie czyni.
No i skąd u wszystkich to new EventArgs. Przecież od tego jest EventArgs.Empty.
Paweł
Hello
“Ja preferuje przypisanie handlera do zmiennej lokalnej, dzięki temu nie złapiemy się na null reference exception w przypadku wielu wątków”
Mozesz podac wiecej szczegolow?
“Tworzenie domyślnej wartości ma swój narzut co w przypadku posiadania dużej ilości obiektów jest odczuwalne i ja go nie stosuję, bo 1 if krzywdy nie czyni.”
To chyba nie istotne dzis…
“No i skąd u wszystkich to new EventArgs. Przecież od tego jest EventArgs.Empty.”
A jakie sa zalety tego?
Pozdrawiam
Piotr
var handler = Handler;
if (handler != null)
handler(this, EventArgs.Empty);
Tak zalecają specjaliści jak Skeet i inni.
Co do nieistotności narzutu pamięci to niby dlaczego? Dlatego że ją mamy? Albo piszemy coś dobrze i zgodnie ze sztuką. Poza tym przypisanie pustego obiektu nie zabezpiecza przed problemami z tego co piszą tu: http://stackoverflow.com/questions/786383/c-sharp-events-and-thread-safety
A zalety EventArgs.Empty są takie, że wyrażają intencje. Mówi jawnie – Tak, w tym miejscu chce puste EventArgs’y.
Paweł
@Pawel:
Wlasnie chcialem uniknac Twojego rozwiazania. Co prawda jest teoretycznie thread-safe (nic sie nie wywali) ale wciaz zdarzenie moze zostac wywolane w momencie kiedy nie powinno. To nie sa atomowe operacje
Co do pamieci – to przesada. W wielu miejsach stosujemy null object pattern – np. w EventArgs.Empty, ktore zaproponowales.
A co do wlasnie EventArgs.Empty to OK – nie mam zarzutów mozna korzystac z Empty jak komus czytelniej:) Osobiscie mi to nie robi roznicy ale moze jest to dobry practise aby trzymac sie EventArgs.Empty…
Różnica pomiędzy EventArgs.Empty a przypisaniem public event EventHandler EventHappened = delegate { } jest taka że EventArgs.Empty jest jeden statyczny natomiast w drugim przypadku EventHandler jest tworzony dla każdej instacji obiektu. Zgadzam się natomiast z Piotrem że sztuczki ze zmienną w przypadku współbieżnego przypinania i odpinania obiektu nie są najlepszym pomysłem. W przypadku silnego zagrożenia ze strony wątków pobawiłbym się z lockiem na tym samym obiekcie w metodzie OnEventHappend, oraz w akcesorach add i remove Eventu.
Mozesz stwrozyc rowniez statyczna metode i podpiac ja do EventHappened. Jednak przeciez to zadne obciazenie dla aplikacji biznesowych. To tylko wskaznik na pusta funkcje…
A co do lockow, to powinnismy przeciez ich unikac. Lock to duzo wieksze obciazenie niz domyslna implementacja…