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

Question

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!

Rothwell

What you should describe in detail is in what cases exactly you would like Flush to be called. That's the only important question. My guess is, either "on each call", or "every now and then", and a final flush on dispose. – Announce

Why are you associating a Flush() with garbage collection? I mean, you could call Flush repeatedly during a socket session with a remote server and the only thing you're intending there is to send the message across the wire to the other machine... – Nuss

@Groo. To some extent, it doesn't matter when Flush has to be called. My concern is that I don't want the fact I have to Flush (whether occasionally or on application shutdown) to leak through into my ITracker interface. – Rothwell

Maybe it is also option for you to implement some kind of buffering inside Track() method of your concrete ITracker implementation, so that it serves itself and checks time since last send or track call count or track data size and decide is it time to Flush – Katelynnkaterina

Answer 1

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.

Baht

This is clearly a case where IDisposable will keep the contracts clear. – Nuss

Thanks for your answer. This will work and isn't a bad solution, but I was hoping that there might be another approach that would mean I could avoid changing my interface. On the other hand, perhaps my interface is simply incorrect and needs to change to properly reflect the abstraction at hand. – Rothwell

@Gibbonfiend You can't really think of a way without changing your interface, since your interface was violating srp from the very beginning, imho. – Baht

Answer 2

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.

Portly

+1 nice answer. However, this opens a room for another layer (ITrackerManager) which should decide when to begin/end tracking and which trackers to use. That might be going a bit beyond OP's requirements. – Baht

Thanks for your detailed answer plinth. I think you and @Baht are correct that I simply need to modify my abstraction. I particularly like your explanation of "leakyness": A trivial implementation is not a leaky abstraction. A leaky abstraction is one that doesn't work for all implementations. – Rothwell

A problem with this approach is that it introduces temporal coupling. BeginTracking must be called before Track, which must be called before EndTracking. The interface has no way to enforce the way in which it is used. I've opened up a separate question for this: #35895597 – Rothwell

The temporal coupling here is obvious and because the interface is simple, you would deserve a dope slap for screwing it up. If you're doing something more complicated (for example, an OCR engine), it helps to burn the process into exactly one place (maybe abstract base class). – Portly

Answer 3

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.

Sunbathe

Doesn't this force implementations that have nothing to dispose to implement a Dispose() method? I.e. your, inherently abstract, interface is making concrete decisions. – Carpentaria

Look at IObservable an IObserver interfaces, they might fit well. – Preschool

Answer 4

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.

Pigg

Too much work for a single class. That violates SRP. Your described flow is correct however in that case you need ITrackerManager which takes care of when data should be flushed. – Baht

I think your first paragraph highlights the core of this question. A consumer of the interface shouldn't have to worry about flushing or deciding when to flush; this should be the job of the implementation. – Rothwell