10
\$\begingroup\$

I'm working on a booking application and am in the process of porting some legacy code to a more SOLID and testable architecture. Right now I'm working on the process of canceling bookings. Over the years, we have changed the steps involved here. The core has been the same (update booking entity status, make the timeslot open again, etc), but we've been adding and removing some other steps, and we do some steps depending on where the cancellation is made from (support UI, customer UI etc).

My current solution involves the decorator pattern for dynamically adding extra steps to the cancellation process. In order to hide the concrete classes responsible for those steps, as well as the fact that this pattern is used at all from client code, I've wrapped this up in a builder. I now wonder if there are some pitfalls I should look out for here.

public class OrderCancellationProcessBuilder
{
    private readonly ITimeProvider _timeProvider;
    private IOrderProcessor _cancellationProcessor;

    public OrderCancellationProcessBuilder(ITimeProvider timeProvider, string cancelationMessage)
    {
        _timeProvider = timeProvider;
        _cancellationProcessor = new CancelBookingsAndRevokePayments(cancelationMessage);
    }

    public IOrderProcessor Build()
    {
        return _cancellationProcessor;
    }

    public OrderCancellationProcessBuilder SendConfirmationEmails(IConfirmationEmailSender emailSender)
    {
        _cancellationProcessor = new SendConfirmationEmails(
            _cancellationProcessor,
            emailSender,
            _timeProvider);
        return this;
    }

    public OrderCancellationProcessBuilder RemovePendingMessages(IPendingMessagesService pendingMessagesService)
    {
        _cancellationProcessor = new RemovePendingMessages(_cancellationProcessor, pendingMessagesService);
        return this;
    }
}

The order processor can later on be consumed by the client code by something like someOrder.Accept(cancellationProcessor);. SendConfirmationEmails and others are the decorators for the order processor. The decoration steps are being hidden behind the builder pattern in a 1:1 way (although this could of course change).

If I were to voice any concern I might have at this stage, it is that it might be overly complicated. But then again, I think this way of thinking might come partly from the fact that the current code LOOKS pretty simple until you look into things and realize how much global state and newing of dependencies there are. If this looks complicated, it might just be because it IS complicated, and this pattern just makes it more visible.

I really think something like this solution could work to make it a lot easier to add/remove steps without breaking existing code, but I wonder if there's a simpler way

\$\endgroup\$
  • \$\begingroup\$ Have you tried making the OrderCancellationProcessBuilder an interface since it gets used to build different OrderCancellationProcess \$\endgroup\$ – Siobhan Jul 13 '16 at 17:20
1
\$\begingroup\$

You can possibly try Domain Events. Or just Event Broker patterns?

What I usually use is something like this:

static class Event
{
   public static void Raise<T>(this T e) ...
}    

interface IHandler<T>
{
   void Handle<T>(T e);
}

So I can define in my code:

class OrderCancelled
{
   public OrderCancelled(int orderId) ...
   public int OrderId { get; }
} 

class MoneyReturn : IHandler<OrderCancelled> ...
class LastOfferSender : IHandler<OrderCancelled> ...
etc...

My IoC container was setup to construct and invoke a transient instances of all implementations of IHandler<T> on a T event raising.

new OrderCancelled(id).Raise();

You can chain events.

\$\endgroup\$
  • \$\begingroup\$ We don't really have an IoC container in use right now, up until recently the modus operandi has been to douple everything to concrete types that you new up wherever you need them. Would you say this approach is viable and significantly better than the proposed solution, even without the auto-wiring of an IoC container? Because it doesn't feel like it to me, but I haven't really worked with domain events. \$\endgroup\$ – sara Jan 26 '16 at 16:00
  • \$\begingroup\$ We should have something like Event.Subscribe<T>(Action<T> handler) and Event.Unsubscribe<T>(Action<T> handler), but I would not use it much outside of unit tests. Maintaining subscriptions becomes a total mess really quick. The IoC container driven approach from above is a really powerful though and for now has no real pitfalls for me. I would suggest to give domain events a try. You can use IoC container for event dispatcher, nothing else, to minimize modifications. I can share my event dispatcher if you wish (tested with Autofac). \$\endgroup\$ – Dmitry Nogin Jan 26 '16 at 16:17

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.