Question

I'm building a small reusable library for two systems our company manages.

Something that I've been caught up on is whether I should expose a set of properties of type Action<T> for events such as Completed, Aborted, ExceptionOccured, or if I should use the Event<T> T : EventArgs methodology.

In practice another internal library will be consuming this information and handling the display of information to the user. The library's classes will be instantiated within a single thread and wouldn't be consumed by anything outside of its owner.

To me it seems more along the lines of if I only need to notifications are only ever going to be consumed by one object then it doesn't matter, however for completeness, and due to the fact that I as the library auther have little control over how another developer implements this, I should use the Event method due to it's broader ability to handle events.

EDIT (sample code):

@Robert Harvey, I read that post when searching for reasoning on picking one over the other. I understand the answer in context to that question, but I'm not sure how it relates to using the event keyword or the simple assigning of a property.

@Fabio Marcolini, I understand that I'm using the word event to describe what is happening. What I'm trying to answer is that if using the event structure is preferred to simply executing a delegate property.

Here are the properties that I use to define the event actions:

public Action<WorkflowInstallerCompletedEventArgs> InstallCompleted;

public Action<System.Activities.Activity, WorkflowInstallerActivityCompletedEventArgs> ActivityCompleted;

public Action<System.Activities.Activity, WorkflowApplicationAbortedEventArgs> InstallAborted;

public Func<System.Activities.Activity, WorkflowApplicationUnhandledExceptionEventArgs, UnhandledExceptionAction> UnhandledExceptionDuringInstall;

The idea would be to invoke one of them in this manner:

  if (this.InstallCompleted != null)
    this.InstallCompleted(new WorkflowInstallerCompletedEventArgs(logs));

Now until @svick's comment I would've said that you could only have one Action (or Func) assigned to each property.

Example implementation:

  WorkflowInstaller installer = new WorkflowInstaller(activity, new TestVariablesCollection()) { ActivitiesRunSequentially = true };
  installer.AddInstallActivity(activity2);

  installer.InstallCompleted = (e) =>
  {
    Log[] logs = e.Logs.ToArray();
    string test = SerializationHelper<Log>.Serialize(logs.FirstOrDefault());
    try
    {
      logOutputFromRun = SerializationHelper<Log[]>.Serialize(logs);
    }
    catch (Exception ex)
    {

    }
    syncEvent.Set();
  };
  installer.InstallAborted = (a, e) =>
  {
    Assert.Fail("Unexpected abort during the install.");
    syncEvent.Set();
  };
  installer.UnhandledExceptionDuringInstall = (a, e) =>
  {
    Assert.Fail("Unexpected exception during the install.");
    syncEvent.Set();
    return System.Activities.UnhandledExceptionAction.Terminate;
  };

  installer.StartInstallation();

  syncEvent.WaitOne();

The answer to my question may not be a definitive this way or that way, but more of best practice or generally accepted approach. I'm really trying to expand my horizons when it comes to creating code that is acceptable to a wider audience.

Was it helpful?

Solution

If it's truly meant to be a library, then you should be using Events. For the reasons Fabio mentioned (nomenclature and looking at existing .NET libraries), as well as the practice that consumers should not have to know or care about each other.

What do I mean by this last statement? Well, consider the following case, where you are using a shared instance of WorkflowInstaller installer:

Class 1 wants to implement your event/action as an Action:

installer.InstallCompleted = (e) => { /* do something */ );

Now, Class 2 also wants to implement your event/action as an Action:

installer.InstallCompleted = (e) => { /* do something else */ );

Well, now Class 2 has just overwritten the handler of Class 1. The action from Class 1 will never get called, causing unexpected behavior.

Implemented as an event, however, this would instead be:

Class 1 implements the event:

installer.InstallCompleted += (e) => { /* do something */ };

Class 2 implements the event:

installer.InstallCompleted += (e) => { /* do something else */ };

Now, of course, we can get around the overwrite in the Action case by checking if any implementation is already in place:

Class 2 implements the action:

if (installer.InstallCompleted == null)
{
    installer.InstallCompleted = (e) => { /* do something else */ };
}
else
{
    var existingAction = installer.InstallCompleted;
    installer.InstallCompleted = (e) => { existingAction(e); /* do something else */ };
}

So you see, we can get around it, but we are forcing consumers to be aware of any other consumers. We are making a larger coding effort on anyone down the line. This is kind of counter to the reason for making a library in the first place.

On a final note, with all the above being said, it doesn't particularly matter as much in your case, as you've said that both the library and consumers will be internal to your workspace. However, as you've already said, in the interest of completeness, I would recommend events.

Licensed under: CC-BY-SA with attribution
scroll top