Ayende @ Rahien

Hi!
My name is Oren Eini
Founder of Hibernating Rhinos LTD and RavenDB.
You can reach me by phone or email:

ayende@ayende.com

+972 52-548-6969

, @ Q c

Posts: 18 | Comments: 87

filter by tags archive

Limit your abstractionsApplication Events–Proposed Solution #1

time to read 4 min | 611 words

In my previous post, I explained why I really don’t the following.

image_thumb[3]

public class CargoInspectionServiceImpl : ICargoInspectionService 
{
  // code redacted for simplicity

 public override void InspectCargo(TrackingId trackingId)
 {
    Validate.NotNull(trackingId, "Tracking ID is required");

    Cargo cargo = cargoRepository.Find(trackingId);
    if (cargo == null)
    {
      logger.Warn("Can't inspect non-existing cargo " + trackingId);
      return;
    }

    HandlingHistory handlingHistory = handlingEventRepository.LookupHandlingHistoryOfCargo(trackingId);

    cargo.DeriveDeliveryProgress(handlingHistory);

    if (cargo.Delivery.Misdirected)
    {
      applicationEvents.CargoWasMisdirected(cargo);
    }

    if (cargo.Delivery.UnloadedAtDestination)
    {
      applicationEvents.CargoHasArrived(cargo);
    }

    cargoRepository.Store(cargo);
 }
}

Now, let us see one proposed solution for that. We can drop the IApplicationEvents.CargoHasArrived and IApplicationEvents.CargoWasMisdirected, instead creating the following:

public interface IHappenOnCargoInspection
{
   void Inspect(Cargo cargo);
}

We can have multiple implementations of this interface, such as this one:

public class MidirectedCargo : IHappenOnCargoInspection
{
   public void Inspect(Cargo cargo)
   {
        if(cargo.Delivery.Misdirected == false)
             return;

        // code to handle misdirected cargo.
   }
}

In a similar fashion, we would have a CargoArrived implementation, and the ICargoInspectionService would be tasked with managing the implementation of IHappenOnCargoInspection, probably through a container. Although I would probably replace it with something like:

container.ExecuteOnAll<IHappenOnCargoInspection>(i=>i.Inspect(cargo));

All in all, it is a simple method, but it means that now the responsibility to detect and act is centralized in each cargo inspector implementation. If the detection of misdirected cargo is changed, we know that there is just one place to make that change. If we need a new behavior, for example, for late cargo, we can do that by introducing a new class, which implement the interface. That gives us the Open Closed Principle.

This is better, but I still don’t like it. There are better methods than that, but we will discuss them in another post.

More posts in "Limit your abstractions" series:

  1. (22 Feb 2012) And how do you handle testing?
  2. (21 Feb 2012) The key is in the infrastructure…
  3. (20 Feb 2012) Refactoring toward reduced abstractions
  4. (16 Feb 2012) So what is the whole big deal about?
  5. (15 Feb 2012) All cookies looks the same to the cookie cutter
  6. (14 Feb 2012) Commands vs. Tasks, did you forget the workflow?
  7. (13 Feb 2012) You only get six to a dozen in the entire app
  8. (10 Feb 2012) Application Events–event processing and RX
  9. (09 Feb 2012) Application Events–Proposed Solution #2–Cohesion
  10. (07 Feb 2012) Application Events–Proposed Solution #1
  11. (06 Feb 2012) Application Events–what about change?
  12. (03 Feb 2012) Application Events–the wrong way
  13. (02 Feb 2012) Analyzing a DDD application

Comments

Frank Quednau

That's all fine and such, but you haven't limited your abstractions, you sliced the system differently. I admit it is a more favorable structure for the reasons you state, alas your code also has abstractions. in fact every time you extract a method or introduce a new class you are introducing an abstraction.

Hence I would say it is not about limiting your abstractions, it is about choosing the correct ones.

Ryan Heath

One thing what we miss from the original code is the sequence of event handling. I do not know if it is really important but your (current) solution doesn't safeguard for those situations.

// Ryan

jjchiw

This is like the Chain of Responsability pattern....

Rafal

I don't understand one thing: why do you insist on having the Inspect(Cargo) method? It's unclear who should call it and when and it also shouldn't be called multiple times or event duplication will occur (and it looks innocent and safe to inspect a cargo as many times as you want). Why the event logic isn't triggered by actual cargo events occuring, for example by information about cargo being delivered/lost/mis-delivered/and so on and instead you are relying on analyzing event history during inspection?

Mani

Not sure where the responsibility is pushed to now. Could we have bit more of this code snippet

Craig Wilson

Yeah, not sure this is the correct method. Events should get raised from inside entities/aggregates when it makes sense, not services. In this particular case, there is already a place to raise these 2 events; cargo.DeriveDeliveryProgress.

Ultimately though, when looking at this, I believe the abstractions are just wrong. We shouldn't need to call DeriveDeliveryProgress to figure out that the cargo was misdirected. It seems there are some missing concepts in here. Alas, I'm not a domain expert so I can't say for sure, but this whole bit of code seems odd.

Ayende Rahien

Rafal, Baby steps, I am doing this one step at a time.

Comment preview

Comments have been closed on this topic.

FUTURE POSTS

  1. Buffer allocation strategies: A possible solution - about one day from now
  2. Buffer allocation strategies: Explaining the solution - 3 days from now
  3. Buffer allocation strategies: Bad usage patterns - 4 days from now
  4. The useless text book algorithms - 5 days from now
  5. Find the bug: The concurrent memory buster - 6 days from now

There are posts all the way to Sep 11, 2015

RECENT SERIES

  1. Find the bug (5):
    20 Apr 2011 - Why do I get a Null Reference Exception?
  2. Production postmortem (10):
    03 Sep 2015 - The industry at large
  3. What is new in RavenDB 3.5 (7):
    12 Aug 2015 - Monitoring support
  4. Career planning (6):
    24 Jul 2015 - The immortal choices aren't
View all series

Syndication

Main feed Feed Stats
Comments feed   Comments Feed Stats