Ayende @ Rahien

It's a girl

Limit your abstractions: Refactoring toward reduced abstractions

So in my previous post I spoke about this code and the complexity behind it:

public class CargoAdminController : BaseController
{
  [AcceptVerbs(HttpVerbs.Post)]
  public ActionResult Register(
      [ModelBinder(typeof (RegistrationCommandBinder))] RegistrationCommand registrationCommand)
  {
      DateTime arrivalDeadlineDateTime = DateTime.ParseExact(registrationCommand.ArrivalDeadline, RegisterDateFormat,
                                                             CultureInfo.InvariantCulture);

      string trackingId = BookingServiceFacade.BookNewCargo(
          registrationCommand.OriginUnlocode, registrationCommand.DestinationUnlocode, arrivalDeadlineDateTime
          );

      return RedirectToAction(ShowActionName, new RouteValueDictionary(new {trackingId}));
  }
}

In this post, I intend to show how we can refactor things. I am going to do that by flattening the architecture, removing useless abstractions and creating a simpler, easier to work with system.

The first thing to do is to refactor the method signature:

[AcceptVerbs(HttpVerbs.Post)]
public ActionResult Register(string originUnlocode, string destinationUnlocode, DateTime arrivalDeadline)

Those are three parameters that we need, there is no need to create a model binder, custom command, etc just for this. For that matter, if you already have a model binder, why on earth do you store the date as a string, and not a date time. The framework is quite happy to do the conversion for me, and if it can’t, I can extend the infrastructure to do so. I don’t need to patch this action with date parsing code.

Next, we have this notion of booking a new cargo, looking at the service, that looks like:

public string BookNewCargo(string origin, string destination, DateTime arrivalDeadline)
{
    try
    {
        TrackingId trackingId = BookingService.BookNewCargo(
            new UnLocode(origin),
            new UnLocode(destination),
            arrivalDeadline
            );
        return trackingId.IdString;
    }
    catch (Exception exception)
    {
        throw new NDDDRemoteBookingException(exception.Message);
    }
}

The error handling alone sets my teeth on edge. Also, note that we have a complex type for TrackingId, which contains just a string (there is a lot of code there for IValueObject<T>, comparison, etc), all of which basically go away if you use an actual string. The same is true for UnLocode (UN Location Code, I assume), but at least this one has some validation code in it.

Then there is the lovely forwarding call, which translate to:

public TrackingId BookNewCargo(UnLocode originUnLocode,
                               UnLocode destinationUnLocode,
                               DateTime arrivalDeadline)
{
    using (var transactionScope = new TransactionScope())
    {
        TrackingId trackingId = cargoRepository.NextTrackingId();
        Location origin = locationRepository.Find(originUnLocode);
        Location destination = locationRepository.Find(destinationUnLocode);

        Cargo cargo = CargoFactory.NewCargo(trackingId, origin, destination, arrivalDeadline);

        cargoRepository.Store(cargo);
        logger.Info("Booked new cargo with tracking id " + cargo.TrackingId);

        transactionScope.Complete();
        return cargo.TrackingId;
    }
}

And now we got somewhere, we actually have something there that is actually meaningful. I’ll skip going deeper, I am pretty sure that you can understand what is going on.

From my point of view of the common abstractions in an application:

  1. Controllers
  2. Views
  3. Entities
  4. Commands
  5. Tasks
  6. Events
  7. Queries

Controllers are at the boundaries of the system, they orchestrate the entire system behavior. Note that I have no place for services or repositories in this list. That is quite intentional. Instead of going that route.

Take a look at the code that I ended up with:

 [AcceptVerbs(HttpVerbs.Post)]
 public ActionResult Register(string originUnlocode, string destinationUnlocode, DateTime arrivalDeadline)
 {
     var origin = Session.Load<Location>(originUnlocode);
     var destination = Session.Load<Location>(destinationUnlocode);

     var trackingId = Query(new NextTrackingIdQuery());

     var routeSpecification = new RouteSpecification(origin, destination, arrivalDeadline);
     var cargo = new Cargo(trackingId, routeSpecification);
     Session.Store(cargo);

     return RedirectToAction(ShowActionName, new RouteValueDictionary(new {trackingId}));
 }

As you can see, the entire architecture was collapsed into a single method.

And what kind of abstractions do we have here?

Well, we have the usual things from MVC, Controller, Action, parameter binding.

We have the session that we are using to load data by id, and to store the newly create cargo.

And we have the notion of a query. Generating a new TrackingID is a query that happen on the database (actually implemented as a hilo sequence). That is something that is definitely not the responsibility of the controller action, so we moved it into a query. Note that we have the Query() method there. It is defined as:

protected TResult Query<TResult>(Query<TResult> query)

And NextTrackingIdQuery is defined as:

public class NextTrackingIdQuery : Query<string>

Pretty simple, overall. And I can hear the nitpickers climb over the fences, waving the pitchforks and torches. “What happen when you need to reuse this logic? It is not in the UI and …”

There are a couple of things to note here.

First, there isn’t anywhere else that needs to book a cargo. And saying “and what happen when…” flies right into a wall of people shouting YAGNI.

Second, let us assume that there is such a need, to reuse the booking cargo scenario. How would we approach this?

Well, we can encapsulate the logic for the controller inside a Command. Which gives us:

[AcceptVerbs(HttpVerbs.Post)]
public ActionResult Register(string originUnlocode, string destinationUnlocode, DateTime arrivalDeadline)
{
    var trackingId = ExecuteCommand(new RegisterCargo
    {
        OriginCode = originUnlocode,
        DestinationCode = destinationUnlocode,
        ArrivalDeadline = arrivalDeadline
    });

    return RedirectToAction(ShowActionName, new RouteValueDictionary(new { trackingId }));
}

And then we have the actual RegisterCargo command:

public abstract class Command
{
    public IDocumentSession Session { get; set; }
    public abstract void Execute();

    protected TResult Query<TResult>(Query<TResult> query);
}
public abstract class Command<T> : Command
{
    public T Result { get; protected set; }
}

public class RegisterCargo : Command<string>
{
    public override void Execute()
    {
        var origin = Session.Load<Location>(OriginCode);
        var destination = Session.Load<Location>(DestinationCode);

        var trackingId = Query(new NextTrackingIdQuery());

        var routeSpecification = new RouteSpecification(origin, destination, ArrivalDeadline);
        var cargo = new Cargo(trackingId, routeSpecification);
        Session.Save(cargo);

        Result = trackingId;
    }

    public string OriginCode { get; set; }

    public string DestinationCode { get; set; }

    public DateTime ArrivalDeadline { get; set; }
}

Note that the Command class also have a way to execute queries, in fact, it is the exact same way that we use when we had the code in the controller. We just moved stuff around, not really made any major change, but we can easily start using the same functionality in another location.

I generally don’t like doing this because most functionality is not reused, it is specific for a particular place and scenario, but I wanted to show how you can lift some part of the code and move it to a different location, otherwise people would complain about the “lack of reuse opportunities”.

On my next post I am going to talk about the Query() and ExecuteCommand() methods, and why they are so important.

Comments

Alex
02/20/2012 07:16 AM by
Alex

For that matter, if you already have a model binder, why on earth do you store the date as a string, and not a date time.

Manual datetime parsing is done probably because MVC in some cases uses current culture and in some cases invariant culture. The only way to specify culture is to write own modelbinder or parse date by hands.

Falhar
02/20/2012 07:27 AM by
Falhar

Looks nice. One thing I dont like is mixing business logic with database logic. I would really like to encapsulate work with session into some generic repository, that is not dependent on nHibernate. But I understand you dont like this kind of abstraction.

Othervise, it looks really nice. I like it.

And I really like how you execute queries and commands. I can already see why they are important.

Thomas Eyde
02/20/2012 08:15 AM by
Thomas Eyde

Nice. But I didn't like the command-refactoring, though: The command itself doesn't communicate that the execution result is a new tracking id. Now I have to read and mentally parse the code to find out.

Vaughn Vernon
02/20/2012 08:23 AM by
Vaughn Vernon

This is now a data centric application where any insight into the business domain has been programmed away and replaced with technology. The domain model has been reduced to an anemic reflection of the database.

Rémi BOURGAREL
02/20/2012 08:51 AM by
Rémi BOURGAREL

“and what happen when…” but in some business like the tourism business, you have to think about it, because the next step is "what happens when a partners want to integrate our booking system using web service for his own web site before the next season".

Ayende Rahien
02/20/2012 09:36 AM by
Ayende Rahien

Thomas, I agree, as a command, it seems wasteful. As I said, this is there because people clamor for "reusable logic". But if the command doesn't communicate what it returns, you can also add that to the name: RegisterCargoAndReturnTrackingId

Ayende Rahien
02/20/2012 09:40 AM by
Ayende Rahien

Vaughn, Yes, it is. And that is wonderful. Because in the entire application, I couldn't find something that wasn't strongly tied to data access. So I kicked all the useless abstractions that made it so hard to figure out what was going on and focused on what is actually going on.

Can you show me a piece of actual business logic in the original application? Something that actually do something meaningful, rather than create infinite layers of abstractions in an effort to pretend that you aren't loading/saving to a database?

If/when you do that, we can focus on making sure that this part of the application is express intent clearly. We have no need to burden the entire app with non relevant concepts when all we actually need is CRUD.

Ayende Rahien
02/20/2012 09:41 AM by
Ayende Rahien

Remi, Um... and the problem with that is exactly what? I actually have shown you a refactoring that can take 3 - 5 minutes. From code in the controller actions to code in a reusable command. No need to do that in advance.

Tom Janssens
02/20/2012 09:41 AM by
Tom Janssens

I am getting a lot of value out of these series, as one of my more recent projects clearly was a project having to much abstractions (which was probably the consequence of designing my app against/using interfaces).

One remark: I would pass the queried Id into the handler/command as a parameter, in order to avoid collisions etc... This also allows you to respect CQS, which is always an advantage IMO (i.e. one could turn the command into a task for example).

artr
02/20/2012 09:44 AM by
artr

Remi, the operations exposed via web services usually have a different granularity and interaction model than the interfaces exposed for a controller. So you'll have to modify or write new ones anyway.

Rémi BOURGAREL
02/20/2012 09:59 AM by
Rémi BOURGAREL

@Ayende, here the problem is simple but in a real world scenario, you'll have to manage availability searching, insurance, options, client's details, commission, emailing ... so your 5 min migration might become a few months and you'll also have to change all your unit test. And do again all your Q&A test.

The problem here is that you say that since it's only doing an insert it's a CRUD, but here this operation is not about data management or maintenance (which in my mind is the purpose of CRUD), it's about making money from client, so in this perspective it IS business logic, a very simple one but still.

Frank Quednau
02/20/2012 10:15 AM by
Frank Quednau

Thomas, ayende,

for that reason of communicating intent I would actually keep a TrackingId class, as throwing a tracking id around as string smells to me like primitive obsession. If the command returns a tracking id, everything is clear. Nobody stops you from having implicit operators that convert to/from string and a decent representation via ToString().

Also the (string origin, string destination) is quite obviously a Leg.

Kill abstractions, fair enough, but do we have to go back to procedural programming? In my experience, a nice sprinkle of value objects definitely aids in understanding the code, the structural dimension of OO is not your enemy.

Falhar
02/20/2012 10:19 AM by
Falhar

I agree with Rémi and Vaughn. Key point of Domain-Driven development is to be able to work with domain without thinking about who or how the code will be called. Different team of people can work on domain and different people can work on webservices or controllers that will expose this domain to outside world. I can also think about Domain without thinking how it will be saved into database or how it will be accessed by outside world. If it will be accesed by local website or through WCF or consumed directly by WPF.

I agree this might seem as needless abstraction, but I believe this kind of abstraction is good and necessary kind of abstraction.

tobi
02/20/2012 10:20 AM by
tobi

I could not agree more. Add abstractions as you need them. It doesn't take long once the need arises.

But why the commands?

Duke
02/20/2012 10:25 AM by
Duke

Great series. You should have some way to link these posts together like a related blog item concept. So if you tune in late or just happen to stumble across a post via google you can easily navigate to other posts from this one.

Ayende Rahien
02/20/2012 10:28 AM by
Ayende Rahien

Falhar, I would strongly disagree with those statements. If you don't care what is going on, you are going to create piss poor software. See all of my previous series talking about software that is highly abstracted and the results of such code.

You really care about the context in which you code is used.

Ayende Rahien
02/20/2012 10:28 AM by
Ayende Rahien

Tobi, Because I wanted to show how to encapsulate that logic, that is all.

Ayende Rahien
02/20/2012 10:29 AM by
Ayende Rahien

Frank, I hear your point about primitive obsession, I am not sure that I agree, but I understand this. I probably wouldn't mind it if it was clearly there to express intent. The problem is that often to create a proper primitive you have to do a LOT of work, most of which isn't really worth it. (Equality, operators, hash code, etc)

Falhar
02/20/2012 10:32 AM by
Falhar

So you are saying, everything said in this: http://books.google.cz/books/about/Domaindrivendesign.html?hl=cs&id=7dlaMs0SECsC book is wrong?? Did you actually read it?

Ayende Rahien
02/20/2012 10:35 AM by
Ayende Rahien

Falhar, I read it, I liked it, and I actually paid attention. You should use DDD in complex projects where the cost of DDD actually make sense. And even then, you should focus on the parts of the application where the business value is. CRUD has very little business value.

In Evans' terms. If your core business is being able to handle re-routing midway, this is where you would use DDD. If your code business is being able to stream line the process of getting cargo out of customs, that is where you focus your efforts with DDD and the whole weight. You don't use DDD for the "enter new cargo info" screen. That isn't your core business.

tobi
02/20/2012 10:39 AM by
tobi

Ayende, I don't use primitives much either, but R# can generate a lot of the default members for you.

There is also a trick to reuse work for ID primitives. You create a wrapper-struct called ID. And you use it like ID and ID. The type parameter is unused. It just serves to make the types incompatible.

I don't use this.

tobi
02/20/2012 10:46 AM by
tobi

What I meant was: ID[T], ID[Customer] and ID[Order]. I always forget the issue with the angle brackets.

Falhar
02/20/2012 11:05 AM by
Falhar

You might be right, that your example doesn't actually have any complex domain logic. Then I would like to see example that has complex domain logic. I would like to see whole NDDD sample application rewritten in a way you would deem acceptable. And I will bet, that your will end up with code of similiar abstraction and complexity as current NDDD sample. Otherwise, you would be breaking whole Domain code.

Kirill Fox
02/20/2012 12:05 PM by
Kirill Fox

Hi Ayende Rahien, interesting post, have a question: In case if we have Entity Framework as ORM how can we test this Register action. Should we mock Datacontext (which as I understand will be used instead of Session object in this sample) or you do not suppose to test it at all?

Ayende Rahien
02/20/2012 12:08 PM by
Ayende Rahien

Kirill, You can do that using an in memory database.

tobi
02/20/2012 12:20 PM by
tobi

Ayende, any thoughts on using a physical database? That way you have exactly the same behavior.

Ayende Rahien
02/20/2012 12:21 PM by
Ayende Rahien

Tobi, That depend on your need, preferences, etc. I run my tests against an actual database in many cases.

Rowen
02/20/2012 12:53 PM by
Rowen

Enjoying these posts and really happy to see some good examples - thanks Ayende. I do like the simplicity of it, and agree many projects have a lot of what is essentially CRUD and are over-engineered. Would love to try this out on a new project. I am uncomfortable with the unit testing aspect as you essentially end up testing the whole stack rather than isolated components, but given that the stack should be smaller/less complex, I can also see advantages to this. Good stuff to think about!

Ben Fulton
02/20/2012 01:07 PM by
Ben Fulton

The fact that Session is public settable on Command makes me a bit nervous. Wouldn't it be better to make that immutable in some way?

Wayne M
02/20/2012 01:07 PM by
Wayne M

I'm interested to read about this "Query" pattern you're using. I think I kind of get what it does (it encapsulates the calls to your ORM or whatever in reusable chunks?) but it's a bit confusing having never encountered something like that before. Definitely want to know more about that.

Ayende Rahien
02/20/2012 01:10 PM by
Ayende Rahien

Ben, Do you have a common issue in your codebase, where properties just get set by a random and unknownable force?

Ayende Rahien
02/20/2012 01:11 PM by
Ayende Rahien

Wayne, The series ain't over yet :-)

Agata
02/20/2012 01:19 PM by
Agata

I hope you don't mind writing to you. My name is Agata and I am managing editor at iCoder Magazine. The iCoder Magazine is online, weekly magazine devoted to iOS applications development solutions. I'm writing to you to talk about our mutual editorial cooperation on special issue on Programming Apple mobile devices.The magazine will be about 50 pages (PDF issue), promoted on our website and in our newsletters and you can be a part of it. Just let me know what you think. I look forward to hearing from you. Best regards, Agata

agata.izykowska@software.com.pl

Ayende Rahien
02/20/2012 01:23 PM by
Ayende Rahien

Thanks, but that seems hardly relevant to this blog.

Ben Fulton
02/20/2012 01:25 PM by
Ben Fulton

Yes, I do. Sometimes I call them "Junior Programmers" and sometimes I call them "Architects" but the result is about the same :) It would generally occur in an attempt to generalize the command structure across multiple UI's, say a Fitnesse integration test suite, a web application, and an automatic service.

gregory
02/20/2012 01:35 PM by
gregory

I've been enjoying the series and sharing it with others. Something was bothering me about the code for this installation . . .

The use of the database infrastructure in the action method? No, not an issue - just a CRUD action.

The creation of two objects in the action method (coupling the method to the constructor methods of those objects)? Again no, it's a CRUD method so, meh.

Wait, it's a CRUD method! And, having read the book, I believe the system should not 'just' create a route specification and cargo without ensuring the business rules are followed.

O.k., now I'm happy again. I think what Falhar and others are wanting in the code is that DDD business logic. The cool thing about your example is you've shown - very clearly - there is no business logic whereas the Java sample you were reviewing used 'fancy' (useless) abstractions to make it seem like something meaningful was being done without any real business logic. My guess is, the cargo object creation would remain CRUD in an action while the route specification would be a task (guessing it wouldn't require a command) in another method.

Thanks for the posts!

Peter M
02/20/2012 01:54 PM by
Peter M

Fantastic post Ayende! Thanks for continuing this series and responding to all the comments. For other readers who want to see more of how Ayende approaches a problem in code, you should check out his Tekpub video "Triage!" Very informative.

Anrew Gunn
02/20/2012 02:53 PM by
Anrew Gunn

What about validation? Where does this go? Or will we see this in a future post?

Ayende Rahien
02/20/2012 03:02 PM by
Ayende Rahien

Anrew, What type of validation? Parameter validation is handled via an OnSave hook, business validation doesn't exists for this scenario

Wayne M
02/20/2012 04:10 PM by
Wayne M

What I find most interesting to note is that the sample was originally Java and done as an example of following the practices outlined in Evans' DDD book (repositories, services, domain language and all of that jazz) but here Ayende is basically saying there's no need for that because the sample application has no specific domain logic (correct me if I'm wrong on that) and doesn't even need to be following DDD in the first place?

What I find most intriguing about this series (and the one before it, about the Northwind Starter Kit) is that I enjoy the DDD concepts but I'm unsure of where to actually apply them so I lean towards "everywhere" which is obviously wrong. Ayende's recent series has started to open my eyes to the fact that DDD cannot and should not be applied all over just to point and say "Look, DDD!" the same as abstractions shouldn't be used for the sake of using abstractions (e.g. things like a generic repository "just in case" you might want to switch your ORM which rarely if ever occurs in the real world), but should carefully be considered where they are appropriate. While I definitely agree with that, it just feels "strange" because it seems to counter some other advice I've seen which amounted to "abstract everything, whether it's needed or not" and "use service layers everywhere, NEVER have your Controller use an ORM directly". I find this advice much more in "touch" with how Ruby on Rails works - the idea that it's okay for your controller to use your ORM directly without having IRepositories and CustomerRepositories and OrderRepositories providing nothing more than a wrapper around the call to the ORM, it's okay to have only enough abstractions as you really need, and less with the Java-influenced "architecture to the moon" approach which is admittedly much more common.

Bystrik Jurina
02/20/2012 05:28 PM by
Bystrik Jurina

So many people are missing notice and important point in Evans book that pure and strict DDD is appropiate in less than 10% of all applications. Also only few people deeply understand what is DDD about...

Vaughn Vernon
02/20/2012 06:30 PM by
Vaughn Vernon

Ayende, it appears to me that you have basically chosen a Hexagonal architecture, which makes things really simple. I like it. You just have an Outside and an Inside. In this case your Outside is an HTTP client that takes the request from the HTTP Port and Adapts it to something the Inside--the RegisterCargo Command--can use. Perfect. I promote Hexagonal for that very reason.

My concern is not about the architecture. My concern is that you are sending a message that Ask-Decision-Set is a better pattern than Tell-Don't-Ask. I do think that your example is possibly borderline, but I think it is just "domain-y" enough to tuck away the Cargo creation behind a really simple abstraction. Maybe not in this case, but certainly for others.

So if you view each Command handler as an Application Service--and I think they are and that it is not wrong to call it such--the Command handler should be primarily concerned with setting up the execution environment for the Domain Model. That's because I strongly believe that the Command/App Service should not be responsible for domain logic. And regardless of your argument against it, the code in the body of your Command handler is domain logic.

What I am saying is, chances are very good that using one simple abstraction--whether a Domain Service or a Factory--to do the creation you have done, is not an abstraction complexity at all. In fact, even if the Factory or Domain Service returned the new Cargo to your Command and the Command saved it, fine. But an important set of relationships used to define a Cargo don't slip out of the model.

The point is, even if DDD is best used in only in limited areas of the enterprise, and it should, the Cargo domain model is quite small as it is. I believe that pushing a CRUD agenda at less significant parts of the model missing the point that it is still part of the model. Just because you want to simplify the architecture, which is a good thing, don't discard the baby, or 1/8 of the baby, with the bathwater.

Hence, the specific use case you have chosen plays well into your architecture simplification goals. But I think it could negatively influence a CRUD mindset by implying that the 7/8 of complexity that the model does address should also be treated as CRUD. The point: you don't un-model a true part of the model, use case by use case, just because it isn't complex. *** If the entire Cargo model were just as you paint it in this specific use case then yes, make it all CRUD. But I am afraid that you are misrepresenting the remainder of the model by using an oversimplified example. ***

Ayende Rahien
02/20/2012 07:20 PM by
Ayende Rahien

the code in the body of your Command handler is domain logic.

I don't really see anything of real domain value there, so I'll have to disagree with you there. This isn't really valuable in a way that is specific for a domain.

But an important set of relationships used to define a Cargo don't slip out of the model.

In what context? That is the part that I keep talking about. DDD is only valuable in specifc scenarios for the application, it isn't valuable for the entire application. It isn't valuable for most applications, for that matter, and it isn't valuable for all parts of the application.

That is extremely important, and I see people trying to do over and over again. They are trying to use all the DDD wieght for all things, including things that make absolutely no sense.

But I am afraid that you are misrepresenting the remainder of the model by using an oversimplified example.

Can you show me the part of the code that is the really domain part? Where it is all domain logic and criticially important.

Vaughn Vernon
02/20/2012 07:32 PM by
Vaughn Vernon

You need to implement the entire Bounded Context before you will understand. No one is suggesting that the "entire application" should be implemented using DDD, and that's not even part of the definition. In architectural terms DDD is neutral.

What you are failing to see is that the DDD Domain Model resides at the heart of the application, and is typically a very small part of the application. Yet, it captures the important aspects of the business, and it this case the business of booking and shipping Cargo.

I wouldn't dream of trying to model the entire application with DDD, because it would be a waste of time.

I have to say that your comments concisely indicate that you don't understand DDD. You example of architecture simplification is good, but I am sure you've crossed the line into a realm that you don't understand.

Ayende Rahien
02/20/2012 07:38 PM by
Ayende Rahien

Vaughn, Thanks for being condescending. I am quite aware of how DDD works, and I built several DDD apps. I am not speaking out of ignorance. I am speaking out of real world experience.

It is nice to say that the domain model is small and at the heart of the app, but LOOK at the application in question. There is a huge amount of complexity that isn't helping anything, in the name of "supporting the domain model".

I am sorry, that isn't valuable.

Vaughn Vernon
02/20/2012 07:49 PM by
Vaughn Vernon

I've agree with you, and uncondescendingly at that. But you are confusing architecture with the Domain Model. DDD is architecture neutral.

One of my goals is to eventually produce a Cargo sample that cuts away the layers of complexity surrounding the model. But when I do so I won't mistake the surrounding Ports, Adapters, and Application Services for the Domain Model, and I won't confuse readers into thinking that DDD is something that it's not. In the mean time readers will come here and see your examples and think, "wow, who needs DDD."

You can take my warnings on condescension if you choose, or try to shield readers from my very real professional warnings by doing so, but I think it is fair that your readers not be confused by misdirection on what DDD really is. Fair enough?

Ashraf
02/21/2012 12:47 AM by
Ashraf

Thanks Ayende for the article. I wish those architects who abuse abstractions should read these series. In fact I am using similar design approach in my last few projects. For query, I use similar syntax

// EF 4.1 (hope you get correct format).

mycontext.Query().First(); // default simple query

mycontext.Queries().IsCustomerTaken(); // complex query

Ashraf
02/21/2012 01:20 AM by
Ashraf

previous post typo update ...

// EF 4.1 mycontext.Query().First(); // default simple query

mycontext.Queries().IsCustomerTaken(); // complex query

Joseph Daigle
02/21/2012 01:21 AM by
Joseph Daigle

Vaughn,

I'm not sure I can agree with your conclusions about the implications of Oren's writings in this series. He's actually not once mentioned DDD in the series, except to say the name of the project.

The point of the series isn't to declare anything about DDD, but instead shows how software tends to be over-designed and over-engineered.

Leniel Macaferi
02/21/2012 01:46 AM by
Leniel Macaferi

Ayende,

Very clean and simple codez...

After reading every comment above I can see that the majority of people out there do agree with this simplified view of things.

I do share this very train of thought and currently I'm developing an ASP.NET MVC 4 with RavenDB and the example you showed here fits perfectly in what I'm currently doing. It leads you to an easy to manage code-base and one can produce things in a fast pace...

Why should someone spend "half a day" every day just creating and duplicating code files IThis and IThat, IRepoThis and IRepoThat, IServiceThis and IServiceThat and so on, if you only need to use it now and here and nowhere else? From my experience this is a complete waste of time and focus! In 99% of projects it's just a burden.


I enjoy reading your posts and I'm learning a lot from them...

Thanks God for allowing me to see the real value of things! :-)

P.S.: Sometimes I wonder if you sleep my friend!? =] Because you do produce lots of great codez and written material...

Warm regards from Brazil,

Leniel

Stephen Shen
02/21/2012 02:38 AM by
Stephen Shen

How about this, no abstraction at all? [AcceptVerbs(HttpVerbs.Post)] public ActionResult Register(string originUnlocode, string destinationUnlocode, DateTime arrivalDeadline) { var cargo = new Cargo(); cargo.TrackingID = Query(new NextTrainigIdQuery()); cargo.Origin = orignalUnlocode; cargo.Destination = destinationUnlocode; cargo.ArrivalDeadline = arrivalDeadline; Session.Store(cargo);

 return RedirectToAction(ShowActionName, new RouteValueDictionary(new {trackingId}));

}

Smell? If you say there is no bus rule in this case, this might be the way to go.

If you do code like this for CRUD, and then put in more abstraction when there is complex rule, you might end with more coding styles in same application.

Jörgen Andersson
02/21/2012 06:42 AM by
Jörgen Andersson

Hi, I think most of what you've done makes sense. I don't know why the example looked as it did, a few too many layers for my taste. However, I have a few concerns with your simplifications.

  1. I think you should avoid mixing different types of infrastructure code like Db-access with HTTP-request handling and with business logic. Repositories make sense and provide separation of concerns for testing and future development. That is a good trade off, I think.

  2. The fact that a new Cargo should have a new trackingId is business logic to me and makes sense pushing into a factory or a domain service. What about the RouteSpecification that emerges from nowhere?

  3. Wrapping ids and other magic strings in immutable value objects named by the domain is a goid way to increase clarity and get more out of compiler and code completion. At least if using a statically typed language. In addition those classes that seem simple from the start often get extended with more functionality in future. Yet another good trade off and not YAGNI as I see it.

Harry Steinhilber
02/21/2012 07:03 AM by
Harry Steinhilber

@Jorgen

  1. Repositories tend to cost you more than they are worth. You lose all the strengths of your OR/M, which Ayende has discussed ad nauseam.
  2. I'm not sure I'd agree. The fact that Cargo requires a trackingId is covered by the constructor requiring it. Why do you need a service to the same thing a constructor can do?
  3. I agree that wrapping up arbitrary strings in value objects can be worth while, but it does come at a cost. Having to override Equals, GetHashCode, ToString, add == and != operators, implementing interfaces like IComparable, etc. It can get to be a maintenance nightmare for a marginal gain when you have a lot of these classes. In short, use it if it makes sense, but in general YAGNI.
Falhar
02/21/2012 07:23 AM by
Falhar

@Harry Single generic repository can take care of everything. No need to create repository for each agregate root.

The thing here is NEW TrackingId. How does constructor ensure, that TrackingId is unused or in correct format?

And those things implemented by value objects can be abstracted into abstract generic class or code-generated.

Anrew Gunn
02/21/2012 08:47 AM by
Anrew Gunn

I'm concerned with all types of validation. Obviously, user input needs to be validated first, but what about global (more business like) validation. The easiest example would be preventing a new user from registering with a username that's already in use. How do you normally present these errors back to the user?

I'm not familiar with the OnSave hook for validation. Do you treat you domain models like property buckets and allow for them to be in an invalid state and then query them to get the validation errors?

Ayende Rahien
02/21/2012 09:50 AM by
Ayende Rahien

Falhar, Please look at the previous posts I did on this topic. Repository is evil in most cases, and it ends up being horribly costly.

Ayende Rahien
02/21/2012 09:52 AM by
Ayende Rahien

Anrew, Define what type of validation.

Unique user - can only be handled by querying the database. Easiest to do by defining things so creating a new user will fail (unique constraint). No need to handle in the domain.

Property length - no need to handle in the domain.

OnSave hook - In NHibernate, ISaveEventListener, in RavenDB IDocumentStoreListener.

Domain validation is for things like, y

Falhar
02/21/2012 10:50 AM by
Falhar

@Ayende

This is question of separation. You don't mind mixing business and persistence logic. For me, it is given, that they are separate. Repository is key to this. So you break SoC to reduce complexity and remove abstractions.

What if I want to have specific domain action happen or specific exception thrown, when unique constraint fails? Do I have to parse some SQL error to check if it really was this unique constraint, that failed? Same with length constraint.

Ayende Rahien
02/21/2012 10:58 AM by
Ayende Rahien

Falhar, IExceptionTranslator (or something like that) is available in NHibernate and in other contexts as well.

Dmitry
02/21/2012 05:17 PM by
Dmitry

I do something similar for commands but also have the following overload:

void Query<TQuery>(Action<TQuery> queryCreator) { }

which looks like this from the code

var trackingId = Query<NextTrackingIdQuery>(q => { QueryProperty1 = value1; QueryProperty2 = value2; });

It allows you to have use a dependency injection container to create query instances. It is really useful if you have multiple tenants that use different databases with different schema in a single app. The TQuery can even be an interface.

Lev Gorodinski
02/21/2012 07:07 PM by
Lev Gorodinski

My objections to the proposed solution and generally to the limiting of the repository abstraction are not based on DDD, SOA, or any of the other enterprise software methodologies. It is based on an introductory passage from SICP which states that a programming language and the programming "environment" created by the programmer should be a place to organize one's thoughts about the program. Partitioning a program into modules allows a programmer to focus on specific parts of a program without having to consider the entire program at once. This frees mental "RAM". The controller (MVC) has a very specific responsibility - to map user interactions to appropriate handlers for those interactions. It maps raw HTTP messages to Java/C#/Whatever. A map implies the existence of two things, the source (HTTP) and the target (application code), which in turn define the partitions. So even if the controller delegates to a facade (a service, repository, etc) and the facade is used only in one place, it still has value because it allows the controller to focus its responsibilities, and more importantly, it allows the programmer to consider those responsibilities in isolation. The issue then is perhaps a matter of taste. For some the transaction cost of creating the facade is higher than the "RAM" cost of managing the interaction of multiple layers in a single method. Furthermore, the proposed command based solution is no different than delegating to a repository or service. The infrastructure is shifted from being injected into the controller to being inherited from a base controller - a favoring of inheritance over composition. I would however strongly oppose repository implementations with intricate LINQ expression based specification abstractions - these are a prime example of leaky abstractions. Overall, I think there is a balance one must attain in limiting abstractions.

Dan Bergh Johnsson
02/21/2012 07:59 PM by
Dan Bergh Johnsson

OK, a TrackingID is just a string. It has no logic, validation, or limitations that apply to it. Good for you. I hope that is really the case and not just the way the code has been written. This implies that tracking id can be 1Gb long, or it could contain any unicode character. I guess "' OR 1=1 --" is also a valid tracking id. If that is not the case, your code contains a bug with potential security implications (aka vulnerabilities) and you have simplified too much.

Trystan
02/21/2012 08:04 PM by
Trystan

I'm not sure about having Commands execute themselves instead of being passed into a CommandHandler. How do commands get their dependencies? A SendOverdraftNotificationEmail command that executed itself would depend on something to create and send emails. Would that be injected by whatever was creating the command (and if so, how did that get the reference)? Would the command be hard-coded to use certain external systems? When using a CommandHandler, any external dependencies can be injected at the start of the program and then be forgotten about.

Patrik Fredriksson
02/21/2012 09:31 PM by
Patrik Fredriksson

@Jörgen

I'm not familiar with the .Net version but in the Java version the BookingServiceFacade etc is there to illustrate a way of handling remote clients (clients on a different network nodes). The service implementation handles serialization and remoting.

Compare this to CargoTrackingController (https://dddsample.svn.sourceforge.net/svnroot/dddsample/tags/dddsample-1.1.0/src/main/java/se/citerus/dddsample/interfaces/tracking/CargoTrackingController.java) which assumes its deployed in the same JVM instance as the domain classes. Here the layers are collapsed into the controller.

Ayende Rahien
02/21/2012 09:55 PM by
Ayende Rahien

Dmitry, Nothing prevents you from injecting the dependencies into the command, see the previous answer about InjectProperties. I don't like the syntax when you have an additional lambda.

Ayende Rahien
02/21/2012 09:57 PM by
Ayende Rahien

It maps raw HTTP messages to Java/C#/Whatever

No, it doesn't. Assuming ASP.NET MVC, you are working at a FAR higher level than raw HTTP. Trying to pretend that it does and then add abstractions adds needless complexity.

Furthermore, the proposed command based solution is no different than delegating to a repository or service.

Um, not really. It split things up on a SRP lines. You have independent commands, not complex set of interconnected services.

Ayende Rahien
02/21/2012 09:58 PM by
Ayende Rahien

Dan, Feel free to write all of those validations in your code. I have actual business value to deliver.

Ayende Rahien
02/21/2012 09:59 PM by
Ayende Rahien

Trystan, Commands don't execute themselves, they are being executed by the command executer

Lev Gorodinski
02/21/2012 10:28 PM by
Lev Gorodinski

"No, it doesn't. Assuming ASP.NET MVC, you are working at a FAR higher level than raw HTTP."

Sure, ASP.NET MVC itself exposes a far higher level of abstraction than HTTP, nor does it deal directly with HTTP. However the point is still that the framework lives at an application boundary and from a birds eye view, HTTP messages are mapped to application code. The level of abstraction that the ASP.NET MVC framework connects with internally doesn't matter to clients of the framework. If ASP.NET MVC was changed to listen to sockets directly, or even HTTP.sys, the clients wouldn't care.

"Trying to pretend that it does and then add abstractions adds needless complexity."

Adding needless abstractions adds needless complexity, but I don't see the relationship between "pretending" that ASP.NET MVC operates upon raw HTTP messages and adding abstractions.

"It split things up on a SRP lines. You have independent commands, not complex set of interconnected services."

Yes of course "complex set of interconnected services" sounds bad. If instead you replace that with "service" it no longer sounds that bad :). In fact, I think designing a framework for executing commands from a controller seems like a needless abstraction where you can just delegate directly to a facade (service, or repository). If the proposed command model is SRP then so is a facade. The controller's single responsibility is to map external client activity to facades, where facades do whatever needs to be done, be it getting some data from a database, placing a message on a queue, etc. The facade implements the core logic of the application, the controller implements interop.

Dmitry
02/21/2012 10:38 PM by
Dmitry

@Oren,

I try to avoid property injection whenever possible. It makes the classes more brittle because in a multi-developer project it is very likely for someone to add a that does will not be resolved by the container. Also, it exposes dependencies in the interfaces if they are used.

Trystan
02/21/2012 10:39 PM by
Trystan

So I'm guessing that the commands expose their dependencies with properties and whatever implements the ExecuteCommand method uses setter injection to make sure the Command has all of it's dependencies? That means your command executer (domain objects? controllers? other commands?) needs all the dependencies. This would make it easier to implement requirements that depend on where something is being done from ("send an email when doing X from page Y but send a text message when doing X from page Z") but it still seems a little odd to me.

I think I need a more complete example of commands that rely on external services like email, legacy systems, or web services to really get it. I look forward to the upcoming posts!

Dan Bergh Johnsson
02/22/2012 08:49 AM by
Dan Bergh Johnsson

Ayende. Ahh, I see. I stand corrected. I assumed that mitigation of business risks such as security was something your stakeholders cared about. Now I understand they do not, so I assume it is some kind of intranet application. Well, make sure your user interface never leaves the safe harbor of your intranet. The day it becomes publicly accessible, LulzSec will love you.

Ayende Rahien
02/22/2012 09:14 AM by
Ayende Rahien

Dan, The mere fact that you need to worry about data such as: ' OR 1= 1 Tells me a lot about the kind of projects you work with. I wish you all the best, and enjoy Boby's Tables visit: http://xkcd.com/327/

Seriously, you really care about those sort of things? And you call them security issues? In this day & age?

JohnWilander
02/22/2012 09:17 AM by
JohnWilander

Ayende Rahien, your reply "Feel free to write all of those validations in your code. I have actual business value to deliver." actually chocks me. I always say developers really care about quality and robustness. Ignoring well-known security issues will not get you far in this era of software development, IMHO.

lindo
02/22/2012 10:10 AM by
lindo

Lulz. Ayende's own words " If you don't care what is going on, you are going to create piss poor software."...while defending the PISS POOR VULNERABLE SOFTWARE he writes. Its not about you, Ayende, your softwares userbase are subject to attack due to you totally missing the point when it comes to security. But if you want to prove your vulnerabilites are negligable in this day & age, let us know where to find production instances of your almighty software.

Psiinon
02/22/2012 09:54 AM by
Psiinon

Ayende, yes the fact Dan is worried about SQL injection probably tells you a lot about the projects he works with. Real world ones. Ones whose owners dont want them to get compromised. I'm always amazed that SQL injection attacks can still work as there are simple defensive measures that can prevent them, but its still the number one web application security risk in the OWASP Top 10. Your attitude explains how they still occur. Very sad :(

Ayende Rahien
02/22/2012 11:34 AM by
Ayende Rahien

Psiinon, I am the one who is going to create this value. Trying to protect me from my own SQL injection doesn't seems insane to you?

You know what, the code for this blog is public, go ahead and try to find a SQL injection here.

I am taking security very seriously, but do tell me how this thing helps security in ANY way: http://code.google.com/p/ndddsample/source/browse/trunk/src/NDDDSample/app/domain/NDDDSample.Domain/Model/Cargos/TrackingId.cs

Ayende Rahien
02/22/2012 11:36 AM by
Ayende Rahien

Lindo, Do take a moment to relax. Again, the codebase for this blog is openly available. Feel free to try to find a SQL injection issue in here. And I do hope that you aren't really going to tell me that this code adds anything to security, right? http://code.google.com/p/ndddsample/source/browse/trunk/src/NDDDSample/app/domain/NDDDSample.Domain/Model/Cargos/TrackingId.cs

And that is leaving aside the fact that you are talking about a value that is generated by the application. Not user input

Psiinon
02/22/2012 12:04 PM by
Psiinon

Hi Ayende, ok, I will agree that your code looks ok as it stands. However I would still always recommend that you use prepared statements when accessing databases. You never know how your code is going to be used in the future, or whos going to copy it and reuse it for some other purpose. You are writing are popular blog, and so I think you should always demonstrate best practice. Someone could easily copy your code and then modify it for their own purposes, introducing a real SQL injection vulnerability.

Ayende Rahien
02/22/2012 12:12 PM by
Ayende Rahien

Psiinon, HUH?! What on earth are you talking about? Where do you even see the discussion on prepared statements vs. sql concat? Where on EARTH do you see a possible sql injection in the code above.

Thiago
02/22/2012 12:20 PM by
Thiago

Doesn't nhibernate takes care of sql injection issues? I'm not very familiar with it, but googling arround shows me that it takes care of sql injection, as long as you let nhibernate create the queries for you.

Ayende Rahien
02/22/2012 12:23 PM by
Ayende Rahien

Thiago, I have absolutely no idea how the issue of SQL injection came up. But yes, with NHibernate, you don't have to worry about SQL Injections

lindo
02/22/2012 12:44 PM by
lindo

I was merely reacting to your comments re SQLi being a non-issue in this day & age. Extrapolated the VULNERABLE from that. Perhaps loss in translation is a little to blame for my assumption??

Context is important re security AND education. Reading this post alone does not make it clear that this code is part of a specific, open project nor the context in which some inputs are derived/born.

Looks like originUnlocode and destinationUnlocode are more of a worry, upon inspection. Im not versed in c# fortunately.

Erlend
02/22/2012 01:56 PM by
Erlend

Imho, you missed Dan's point. By reducing TrackingId to a string, your code basically ends up saying that a tracking id can be any string. Your code stopped caring about the contents of it. Instead of removing TrackingId, you should augment it to contain any validation rules you have, so that whenever you have a TrackingId-object it's calid according to your domain rules. Thus anywhere else in your code, you don't have to worry about it's contents. As long as you have a TrackingId, you know it's valid. The same holds for UnLocode. Why on earth would you want to send strings around, when you can send proper objects... like DateTime.

Ivan
02/23/2012 01:14 AM by
Ivan

Hey, guys, there's ORM between domain code and actual SQL. It handles this kind of things and much more. What are you talking about here at all?

meisinger
02/23/2012 06:10 AM by
meisinger

wow... this conversation has taken a turn for the worse!

i never thought that i would ever see the day that someone brings up sql injection to the guy who contributed tons of code to NHibernate and created RavenDB

i am actually surprised there was even a response

@Ayende... you are a much more patient person than i am. also let me know when RavenDB has prepared statements... that is all

henginy
02/23/2012 03:45 PM by
henginy

Nice ideas. I would like to ask how .NET 4.0 Tasks would fit in this architecture (Tasks & Commands)? It seems to me they would somewhat overlap. In summary, what do you think of following the same approach with .NET Tasks?

Stephan Hoekstra
04/06/2012 02:01 PM by
Stephan Hoekstra

@Ayende, I have a question about your example of Controllers working with Commands and Queries. - I can imagine that if you have a complex controller which does a lot of things, you would end up having a lot of dependencies. Things can get crazy...

public WorkaholicController( RecoverPassword recoverPasswordCommand, CreateUser createUserCommand, LoginUser loginUserCommand, UserOverViewQuery userOverviewQuery, ResetAllPasswords resetAllPasswordsTask, ... ...) //etcetera, credits for the example to my colleague Rodi ;)

I suppose that would be a bad thing (right? having a lot of dependencies is not something you want.)

The question is: I was wondering how you deal with that in practice? Refactor it into several smaller controllers? Compose the Commands into higher level commands that have subcommands? Is it even a problem?

Ayende Rahien
04/08/2012 10:08 AM by
Ayende Rahien

Stephan, Why would this happen? Look at the code, I am instantiating and creating the command directly, it isn't a dependency that you get in the ctor.

Stephan Hoekstra
04/08/2012 11:09 AM by
Stephan Hoekstra

Well, I am sure that in a real life scenario, CargoAdminController would have more functionality than just Register and BookNewCargo, and it would probably have more action methods and Commands (and Queries, etc.)

Just because CargoAdminController is the logical place for such things, and real scenarios are always more complex than such example cases.

You're right, I did overlook the fact that you do not apply DI for the commands in your example, so you would not have a gigantic constructor signature.

But regardless of how you instantiate these objects, be it via a container framework or "new-ing them up", isn't it true that in both cases, if you have a lot of different command classes that you rely on, then you have a lot of dependencies (which is something you don't want - please correct me if I am wrong).

I suppose I should rephrase my question: Let's say hypothetically, that your example controller grew over time, a number of action methods were added and you ended up with many more (dependencies on) commands and queries...

Would you consider that to be a problem? Is that something you would be ok with, or would you refactor? If you would, then I'm curious to know how.

Ayende Rahien
04/08/2012 11:14 AM by
Ayende Rahien

Stephan, It might be a problem from SRP point of view, but it wouldn't be a problem from any other aspect. It is still just as easy to work with the codebase, and just as easy to modify and change.

Stephan Hoekstra
04/08/2012 11:31 AM by
Stephan Hoekstra

I find this confusing, because I was taught that the Single Responsibility Principle is (always) our friend.

Am I misrepresenting you when I would say that in this case you are basically advocating: - the application is simple enough and it is more valuable to just have code which is easy to work with. And if that violates SRP, too bad - we will deal with that when and IF it becomes a practical problem? - you would only consider refactoring if managing the dependencies becomes so costly (in terms of time) that it makes sense to abstract them away?

Just trying to understand, Thanks.

Ayende Rahien
04/08/2012 12:15 PM by
Ayende Rahien

Stephan, No, I am saying, SRP is orthogonal to the architecture that you are using. My point about SRP is that if the controller has many methods & actions, that is likely to be because of a SRP issue, not related to the architecture. And you shouldn't HAVE that many dependencies that make it complex to handle a single class in the first place

Comments have been closed on this topic.