C# pattern for avoiding a leaky abstraction when one of the implementation requires an extra step

839 Views Asked by At

I am implementing an ITracker interface that looks something like:

public interface ITracker
{
    void Track(ITrackerEvent trackerEvent);
}

I initially created an implementation of this interface wrapping Mixpanel.NET. I then created another one that wraps Application Insights. However, the Application Insights one requires Flush() to send the data to the server.

I don't want to pollute the ITracker interface with a Flush() method just because one of the implementations needs one. It would feel like a leaky abstraction.

However, I need to call this method at some point (probably on application shutdown) and don't want to do so every time Track is called.

Is is possible to call a method when the Tracker is garbage collected at the end of the session? Is this even a good approach?

I feel like I'm missing a trick here!

4

There are 4 best solutions below

3
Leri On

I'd make ITracker to use transaction alike pattern because there may be cases when you might want to discard tracker events, rollback or modify them:

public interface ITracker
{
    void Track(ITrackerEvent trackerEvent);

    void Commit();
}

And then per implementations:

  • I'd call Flush inside Commit for Application insights.
  • I'd write tracker events in in-memory collection (List<ITrackerEvent> or BlockingCollection<ITrackerEvent> if concurrency involved) inside Track method and then use your current logic to call Mixpanel.NET api inside Commit method implementation for Mixpanel.NET.

Recommendation: ITracker should also implement IDisposable since trackers usually use resources that need to be disposed.

4
plinth On

Building on Leri, I would think more along the lines of what a tracker might need to do.

I would be inclined to do something like this:

public interface ITracker {
    void BeginTracking();
    void Track(ITrackerEvent trackerEvent);
    void EndTracking();
}

Then all trackers have a sense of when they're starting and when they're finishing. This is important because a tracker may be holding onto resources that shouldn't be held longer than necessary. If a tracker doesn't need to use either BeginTracking or EndTracking, the implementation is trivial. A trivial implementation is not a leaky abstraction. A leaky abstraction is one that doesn't work for all implementations.

Now suppose you are flat-out against having two extra methods in each tracker (why?). You could instead have ITrackerEvents that are out of band and cover the semantic meaning of Begin and End. I don't like this. It requires every tracker to have special code to handle out of band events.

You can also have a separate interface

public interface IDemarcatedTracker : ITracker {
    void BeginTracking();
    void EndTracking();
}

which requires you to have special case code in your calling code to check to see if the ITracker is also an IDemarcatedTracker:

public void BeginTracking(ITracker tracker)
{
    IDemarcatedTracker demarcatedTracker = tracker as IDemarcatedTracker;
    if (demarcatedTracker != null)
        demarcatedTracker.BeginTracking();
}

And not to over blow things too much, but I would also wonder what is supposed to happen when a tracker fails? Just blindly throw an exception? And here is where the abstraction is actually leaky. There is no process for a tracker to let you know that it can't track.

In your case you might want to return a boolean (limited information), an error code (somewhat more information), or an error class/struct. Or you might want to have a standard exception that gets thrown. Or you might want the Begin() method to include a delegate to call when an error happens in tracking.

2
oleksii On

I'd just derive ITracker from IDisposable. Classes that implement this interface may choose to perform some action in the Dispose method, e.g. your Flush, or do nothing.

public interface ITracker : IDisposable
{
    void Track(ITrackerEvent trackerEvent);
}

Also, have a look at the Observable pattern, the ITracker name suggests that you may want to perform some action when an object's state is changed.

2
Scott Hannen On

It sounds like you're buffering something - the things that get tracked need to get flushed from time to time. Your interface hides that behavior, which is good - you shouldn't be able to tell from that interface when it's getting flushed or even if it's getting buffered.

If it's high-volume I like to set two parameters - a maximum flush interval and a maximum buffer size. The first uses a timer to flush at regular intervals. The second triggers a flush when the capacity is reached. And then I flush again when the object is being disposed or collected.

I separated out the buffer into its own class so I could reuse it and unit test it independently. I'll see if I can find it, but it's not much code to write.