HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Subscribing an Object to its own Events

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
eventsitsownsubscribingobject

Problem

Is it appropriate for the class to subscribe to its own events like this? Should the class not subscribe, and instead move that code to before the OnEventName(Object args) methods call the EventHandlers? I'm curious if there are any downfalls to this approach.

On one hand, with this approach I could remove all the null checks on the event-firing methods. On the other hand, what if another event down the road needs information from the event handling I have already added a subscription to. I.e. in the future a method needs the NetworkServer_Error method to have been guaranteed to have fired before it can work.

I'm mostly concerned with the event logic here, not so much the implementations. None of these implementations are concrete, but I want to make sure I have done this part properly first.

```
public class NetworkServer
{
NetPeerConfiguration mainServerConfiguration;
NetServer mainNetServer;
List connectedClients = new List();

public NetworkServer()
{
mainServerConfiguration = new NetPeerConfiguration("SecretKey");
mainServerConfiguration.AcceptIncomingConnections = true;
mainServerConfiguration.Port = 5501;
mainNetServer = new NetServer(mainServerConfiguration);

// Wire up our events
Error += NetworkServer_Error;
DataReceived += NetworkServer_DataReceived;
StatusChanged += NetworkServer_StatusChanged;
ClientConnected += NetworkServer_ClientConnected;
ClientDisconnected += NetworkServer_ClientDisconnected;
}

void NetworkServer_DataReceived(object sender, NetMessageEventArgs e)
{
Program.LogLine("Data recieved from: " + e.Message.SenderConnection.RemoteEndPoint.ToString() + ", Payload size: " + e.Message.LengthBytes.ToString(), LoggingType.Information);
}

void NetworkServer_Error(object sender, MessageEventArgs e)
{
Program.LogLine(e.Message, LoggingType.Error);
}

void NetworkServer_ClientConnected(obje

Solution

I'm going to suggest "NO" as the answer to this design :)

A few general principles:

  • Events are for exposing lower-level state/change to higher-level actors, and enabling versatility of coupling.



  • Higher-level actor would be independent & in a different class.



  • Here, it's unclear which level of abstraction you are actually trying to build -- is it one coherent level, which shouldn't rely on events, or two separate levels coupled by events, which should be in separate classes?



  • Events don't seem to me a suitable approach for "null checks"/ null handling.



  • Event-handling is not guaranteed as to sequence & it's pretty unsafe/brittle to rely on sequentiality of handling (re: your NetworkServer_Error question near the end).



In general, it's probably best to build a solid mechanism with it's own clear internals, and decouple that from the published API (events etc) visible outwards.

Context

StackExchange Code Review Q#96293, answer score: 5

Revisions (0)

No revisions yet.