1

I really couldn't think of a title which can in several words, describe what I need in more details. Ultimately the idea is that I'm using custom implementation for sending messages to Azure Service Bus. This implementation is wrapped in a NuGet package and I'm trying to add some additional logic which will save the message to the database which is needed for QA automation purposes. The tricky part here is that the I want the NuGet package to remain unchanged and all additional logic to be "wrapped" around it. So the package itself has a lot of sealed classes and internal interfaces but I think I've managed to extract the chain I need. First I have this interface which is used from all classes to publish messages to the Service bus:

public interface IMessageBus : IDisposable
{
    bool Send(IMessage command);

    bool Send(IMessage command, string trackingId);

    Task<bool> SendAsync(IMessage command);

    Task<bool> SendAsync(IMessage command, string trackingId);

    bool Publish(IMessage eventObj);

    bool Publish(IMessage eventObj, string trackingId);

    Task<bool> PublishAsync(IMessage eventObj);

    Task<bool> PublishAsync(IMessage eventObj, string trackingId);
}

it's part of the package but is one of the few public interfaces. Then I have an abstract class implementing this interface:

public abstract class MessageBusBase : IMessageBus, IDisposable
{
    ~MessageBusBase()
    {
        this.Dispose(false);
    }

    #region Interface

    public void Dispose()
    {
        this.Dispose(true);
        GC.SuppressFinalize((object)this);
    }

    protected abstract bool SendMessage(IMessage payload, string trackingId);

    protected abstract Task<bool> SendMessageAsync(IMessage payload, string trackingId);

    protected abstract void Dispose(bool disposing);
}

From here I start to loose the idea behind the implementation. Since IMessageBus already implements IDisposable why the abstract class should implement it again? Resharper is also marking this as not needed, but I've learned not to trust Resharper every time.

So at the end I have sealed class that inherit the abstract MessageBusBase:

public sealed class AzureMessageBus : MessageBusBase
{
    private bool isDisposed;

    #region MessageBusBase

    protected override void Dispose(bool disposing)
    {
        if (!disposing)
            return;
        this.isDisposed = true;
        foreach (IDisposable disposable in (IEnumerable<ISender>)this.senders.Values)
            disposable.Dispose();
    }
}

Having this picture I really would appreciate if someone can explain me what is the purpose of the destructor in the abstract class and why we need it. I've spend some time reading about when and why destructor should be used and the conclusion that I've reached is that actually we most likely don't need it. However in AzureMessageBus are the actual calls to Azure, also the IMessage interface that we are using in the SendMessageAsync method is from Google.ProtocolBuffers and maybe this could be the answer, but now I'm just guessing. So this is the first part of my question.

The second part is adding my additional logic. I did that by creating a new class:

public class AzureMessageBusWithLogging : IMessageBus
{
    private IMessageBus messageBus;

    private IMessagingFactory messageFactory;

    public AzureMessageBusWithLogging(IMessagingFactory msgFactory)
    {
        this.messageFactory = msgFactory;

        if (messageFactory != null)
        {
            this.messageBus = this.messageFactory.CreateMessageBus();
        }
    }

    public Task<bool> SendAsync(IMessage command, string trackingId)
    {
        //Place for my additional logic
        return ((AzureMessageBus)messageBus).SendAsync(command, trackingId);
    }

    public void Dispose()
    {
        throw new NotImplementedException();
    }

    #region IMessageBus
}

Where this.messageFactory.CreateMessageBus(); returns an instance of AzureMessageBus like so:

 return (IMessageBus) new AzureMessageBus(..)

So basically that's all, from my new class I'm calling the original implementations of the methods from AzureMessageBus but since I need to implement IMessageBus in my new class AzureMessageBusWithLogging I'm forced to add :

    public void Dispose()
    {
        throw new NotImplementedException();
    }

But I really don't know what to put inside and if I really need to do something. And also I would really appreciate an explanation why we need this (complicated from my point of view) implementation of GC and how, if in any way the disposing in my new class may affect the existing AzureMessageBus since I'm using instance of it in my new class.

2
  • MessageBusBase just defines an abstract Dispose method which let inherited classes implement their Dispose logic. If there is nothing to be disposed in your implementation, then just leave it empty. Why do you throw an exception (especially assuming that this code will be run by finalizer). Commented Apr 5, 2017 at 10:58
  • @YeldarKurmangaliyev I'm a little bit confused by this design, mostly because it's something new, till now I never spend too much time and thinking on GC. That's why I'm asking this question, not sure what this design is aiming for (if anything) and what we accomplish by implementing it this way. So I would also like to know why is the exception. And thanks for the Dispose answer. I'm also leaning towards just leaving it empty. Commented Apr 5, 2017 at 11:02

1 Answer 1

2
  1. You don't need destructor, because you don't hold any direct unmanaged resources you need to release. If you just wrap some IDisposable objects - just implement Dispose without destructor. If those objects do hold some unmanaged resources - they will have their own destructors.

  2. You don't need to do MessageBusBase : IMessageBus, IDisposable, because IMessageBus already includes IDisposable. Though if you do that - there is no harm

  3. In your AzureMessageBusWithLogging you wrap another IMessageBus messageBus. So in Dispose - dispose that messageBus.

So in general: if you wrap some disposable objects - your class should also implement IDisposable and dispose all those wrapped objects in it's own Dispose, just like you do in AzureMessageBus.Dispose.

Sign up to request clarification or add additional context in comments.

2 Comments

I see, wasn't really sure about that - your class should also implement IDisposable and dispose all those wrapped objects in it's own Dispose. So If I understand you correctly you propose in the Dispose() method to have something like this.messageBus.Dispose()?
That's right. You already do the same in AzureMessageBus (disposing senders) so not sure what caused confusion for the similar case in another class.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.