Update: There’s a wealth of knowledge out there that I just haven’t been aware of until now. Thanks to the many commenters of this post, other posts, and other conversations for the links and info. Here is a list of items that I’m finding to be very helpful in understanding what is only a surface level “validation” problem: Update 3/3/2009 – additional info in the continuing conversation. Update 3/11/2009 There’s a whole host of stuff out there linked from these articles and posts, as well. It’s obvious now that I have a lot to learn and think about, and I think I have a better sense of where I need to start looking. I also think that the ideas Sean Biefeld expressed about never letting an entity be in an invalid state are more reachable than I previously thought.
---------------------------------- This post started as a comment in Sean’s post on Entity Validation Ideation but quickly grew. I’m hesitant to post separately because I believe the conversation should continue along the lines of what he was originally saying. There’s just so much junk tossing around in my head, though, I didn’t think it would be appropriate to post a 50 page diatribe in a comment.  A while back, I asked the question “Where does required info validation belong for an entity?” There were a lot of good answers to that question and my team and I took some very interesting directions based on those answers. However, I still haven’t had the ‘validation’ question completely answered in my mind. Fortunately, I’ve got the brains of my fellow Los Techies (and coworker in this case), to keep me from giving in to the entropy that wants to keep me mediocre. I love the theory that Sean is talking about - especially when stated as 'proactive' vs 'reactive'. I've had a hard time with this in practice, though. In the last few weeks – days, really - it seems to me that we need to have both proactive and reactive validation in our applications. Being Proactive The domain model that I create wants to prevent an object from being invalid, so I require a minimum set of information in my constructors, etc etc. This helps me prevent an object from ever being created in an invalid state. this is the proactive approach. I love this approach and I use it a lot. Oh Wait, I Need Reactive The proactive recently failed me, though. I had ‘the perfect’ domain model with the all the validation and rules composed in my entities, preventing the information from being added to the model if it was not valid. However, when I got to the user interface, I realized that I had a major design problem. The user interface is not 'all or nothing' scenario. The user is allowed (expected, really) to only provide one field of information at a time. After all, you can't type into more than one text box at a time - unless you have multiple keyboards and multi-input capable systems. So, when a user is adding information to the inputs of the form, we need to have a reactive approach. We need to provide real time validation of what the user is putting into the system, so that they can be immediately notified of any errors in what they are providing. This reactive approach is needed because the user interface is entirely reactive to the user's actions. Proactive AND Reactive? At the same time that we need both a proactive and a reactive solution, we want to adhere to the "Don't Repeat Yourself" principle, encapsulating the validation logic into the appropriate location(s). If we require reactive validation in the user interface, and proactive validation in the domain model, how do we avoid breaking DRY? On the surface, proactive and reactive validation seem to be at odds with each other – at least, they do in my mind. I don't think they need to be, or should be, though. We just need to find the higher level order in which we can enable both. Unfortunately, I haven't been able to do this yet – at least not in a way that really seems to satisfy me. I'm curious to know how others approach this situation. How do we balance this triad of need: Proactive, Reactive, and DRY? I need some fresh perspectives on this, and I think Sean is starting out with some great ideas. Now if I can just learn to throw away my own bias to how I’ve already been handling this, maybe I’ll be able to learn something.
_________________________________ Cross Posted From LosTechies.com
In my last post, I talked about the idea of encapsulation and using it to ensure that our business rules were enforced correctly. What I didn't talk about, though, was the second half of the conversation that my coworker and I had, concerning the patent -> consultation relationship. It turns out that we had the relationship wrong. That's not to say that patients don't have consultations, but that the logical model we were traveling with had an incorrect perspective and was causing us to create some very ugly workarounds in various parts of the system. What really stuck out in my mind, though, was not the idea that we had the model wrong, but how we came to the conclusion of the model being wrong. It has become apparent to me, upon reflection of the conversations and situation as a whole, that design smells are not always evidenced by design related activities, if ever. A Persistent Problem - Duplicate Redundancy After some initial coding of the patient -> consultation relationship, we start working on the persistence model via NHibernate. What we have is a patient with a collection of consultations - this is easy to map with NHibernate's one-to-many capabilities. We also have a CurrentConsultation property which needs to be mapped. This property is mapped to the same Consultation table, but only pull one specific consultation based on the business rules that state the current consultation is chronologically the most recent and has not ending date set. After some thought, we found that there were a few possibilities for handling the CurrentConsultation property in our current model: - Create a "CurrentConsultation" object that is mapped to the Consultation table and use a "where" class attribute in the NHibernate mapping that would limit the returned result
- Create a "CurrentConsultation" object that is mapped to a CurrentConsultation view and have the view coded to return the correct consultation object
- Add a CurrentConsultationId field to the Patient table, as a foreign key to the Consultations table, and map to the existing Consultation object
After some additional thought, though, we found that each of these solutions has a few significant problems that were going to cause a lot of trouble. Options 1 and 2 Both of these options have the problem of duplicating business rules into more than one language and location. We would either have the business rules of what constitutes the current consultation in the NHibernate mapping (the 'where' attribute) or in a database view, in addition to the already existing code. Changing the rule would mean changing a minimum of two locations where that rule is handled. This is a bad idea no matter how you look at it. Both of the options have also created a duplication of knowledge from the concept of a Consultation by creating a "CurrentConsultation" class and a separate NHibernate map for it. We would have the original Consultation class and the new CurrentConsultation class both representing the same data, making an artificial distinction in our code. Again, this is a bad idea. We don't want duplication of these logical concepts. We're also not dealing with a bounded context or any other logical separation of concerns at this point, so there is no need to separate the concept of a consultation into multiple classes. Option 3 This doesn't appear to have the duplication issue in code, but there is a potential for duplication of data. When we get down to the implementation of NHibernate, we could easily cause duplicate data in the consultation table by saving the current consultation class. We might be able to get around this by not cascading the saves of the current consultation property, but then we'd be forced to ensure the consultation collection was persisted prior to the patient so that we could update the current consultation object's id before saving the patient. Both of these problems sound like a serious pain to me. I'm betting it's possible, but I'm also betting that it would be a nightmare of trial and error to get it right and a lot more code than we should really have to write. Changing Perspectives As Joe Ocampo pointed out in the comments of my original post, we had a problem in our system that was really caused by our lack of correct perspective in the situation. Rather than forcing the idea of a patient being the root aggregate in this situation, causing us a lot of headache and frustration in trying to model our persistence layer, a simple change in how we looked at the situation helped us solve the persistence problem and greatly simplified how the application worked. Joe's comment (with some formatting added): "One thing I like to challenge developers with when I teach DDD is to flip the aggregate to determine if the model is sound. I know this is only an example but work with me here. You indicated you are dealing with a medical system. We can assume there are certain entities such as Patient, Consultations, Doctor and Practice. In your example you created a model where the patient is the aggregate root for consultations but what if the Doctor simply asked what consultations do I have today? In this paradigm the Practice is the aggregate root and Consultations are aggregate within where Patient is an aspect of the consultation. The code would look something like this. consultations = practiceService(IConsultationService).GetConsultationsFor(doctor);
This also allows the consultation service to encapsulate its own logic for creating a consultation for creating a consultation. You can’t get any closer than that
consultationService.CreateConsultationFor(patient).with(doctor).at(date);
The point I am trying to make is be careful of aggregate roots. Once you go down that path it is really difficult to back the train up and break it apart."
Though our actual implementation was different, this was the same basic conclusion that we had come to - our perspective on the situation was simply wrong. When we stepped back from the problem and realized that the consultation was the primary focus of the situation, and that a nurse or doctor would be the primary user of that portion of the system, it became rather obvious that our aggregate was in dire need of rework.
A Reflective Perspective
What we ended up with was a Patient object that dealt with all of it's demographics information, billing information, etc, without a CurrentConsultation property or even a Consultations list. Then, on the the separated Consultation object, we added a child property of Patient. Once we realized that our Consultation object was the primary focus and made this distinction in our code, we also realized that the Patient object was carrying far too much information around the system. We found that we actually had two very distinct concepts of a patient, determined by two very distinct bounded contexts.
- In the 'Billing' context, we needed all of the address , billing, and other demographics information about the patient - who they are, where they live, what their insurance is, etc. The existing Patient class filled this need.
- In the 'Consultations' context, we did not need anything from the Billing context, except for the person's name and patient id. What we really care about in the consultations is medical information about the patient - their current prescriptions, allergies, past medical care, etc. So, we created a ' patient' class to represent these needs.
These changed allowed for a much more clearly defined model that was truly reflective of the systems needs. We could easily see the difference between a 'billing' patient and a 'medical' patient, and we were able to code each of these areas of the system without the concerns bleeding into each other. Essentially, we decoupled the system at a module level, not just at a class level.
We also found that the NHibernate mapping problems suddenly went away. Since the Consultation class had a child of Patient, it was a simple many-to-one mapping with no strange sequencing or duplicate data issues. In the screens that deal with the consultation directly, we load the consultation as the aggregate root and go from there. In the screens that need to show patient consultation history, we did a simple query and returned all of the consultations for the given patient. Again, we found a way to decouple our system - this time, at the persistence model.
Design Smells: Not Just A Design Problem
In the end, we were able to recognize a serious design smell in our system - not by the design itself, though. After all, the original code had encapsulated the needs quite well. But, as it turns out, it was a bad encapsulation at a higher level. It wasn't until we started working with the model we had created, specifically trying to persist the model, that we realized our design was not right.
This change was a huge breakthrough for us, not necessarily in the code or the system that was being built, but in how we look at our systems and our domain models. The realization that design smells are often evidenced not by the design itself, but by how the design is used in the infrastructure and other supporting roles of the system, has had a profound impact on how we look at system design. I'm now seeing areas of different systems that are encapsulated incorrectly, at a higher level than class design. Recognizing the problem is the first step - and we're now working to rearrange and invert these models to more accurately reflect reality.
Pay attention to the pain that your application, infrastructure and other supporting services are causing you. You may be staring at evidence of a design problem, without realizing it.
_________________________________ Cross Posted From LostTechies.com
Yesterday, I was involved two very separate yet very related conversations. One was via twitter with Colin Jack and Jimmy Bogard (which I was only a partial contributor to - mostly just reading their conversation) and another after work with a coworker. The short version of both conversations can be boiled down to encapsulation of logic surrounding collections that are held by entities. Rather than rehash all of the conversations, I wanted to specifically address a violation of encapsulation that I've seen many times when dealing with collections and business rules. Let's look at a small example where we have a medical system that deals with Patients that are having Consultations with doctors. There is a need to keep track of the historic consultations and also the current consultation, if any. For simplicity we'll say that the current consultation is identified as being the most recent, based on a starting date, and that any previous consultation must have an ending date. A very simplistic implementation of an object model to represent patients and consultations may look something like this: public class Patient { private IList<Consultation> _consultations = new List<Consultation>(); public IList<Consultation> Consultations { get { return _consultations; } } public Consultation CurrentConsultation { get; set; }} public class Consultation { public DateTime StartingDate { get; set; } public DateTime EndingDate { get; set; }}
Then, when the time comes to add a new consultation to the patient, making it the current one and closing a previous consultation, code may get called from somewhere in the application (like a code behind of a form, or if you're lucky, in the Presenter or Controller of an MVP/C setup), like this:
Consultation consultation = new Consultation{ StartingDate = DateTime.Now }; patient.Consultations.Add(consultation); if (patient.CurrentConsultation != null) patient.CurrentConsultation.EndingDate = DateTime.Now; patient.CurrentConsultation = consultation;
From a purely technical standpoint, there is nothing wrong with this code. It implements the business rule as defined. However, this code misses out on some great opportunities to encapsulate the rules we have into a process that can be called from anywhere that the system needs it - whether or not the code is in the specific presenter / controller that creates a new consultation or not. The very same code that comprises this implementation could easily be placed in the Patient object, abstracting the rules and process of creating a new consultation into a simple, single method call.
Before I show how I would approach that solution, though, there is one other implementation that I've seen recently that not only breaks encapsulation, but has to compensate for the lack of rules enforcement with logic in the wrong place. Instead of storing the current consultation as a set value, the CurrentConsultation property may have some logic in it to dynamically determine which consultation is the current one.
public class Patient { private IList<Consultation> _consultations = new List<Consultation>(); public IList<Consultation> Consultations { get { return _consultations; } } public Consultation CurrentConsultation { get { Consultation currentConsultation; DateTime mostRecent = DateTime.MinValue; foreach(Consultation consultation in _consultations) { if (consultation.StartingDate > mostRecent) { mostRecent = consultation.StartingDate; if (consultation.EndingDate == DateTime.MinValue) { currentConsultation = consultation; } } } } } }
This type of code is a huge encapsulation violation smell. Since our Patient object has no enforcement of the consultations that it holds, there is no way for us to really know which consultation is the current one. Because of this, the retrieval of the current consultation has to process the entire consultation collection and try to find the most recent consultation that has no ending date.
On top of the encapsulation issue, we have lost a great deal of performance. We now have to loop through the list every time we need the current consultation. If the list is small, this might not be such a bad problem, but as the list grows and as this code is used more and more, the performance problem may have a serious impact on the system.
Fortunately, the solution to the encapsulation violation, the enforcement of the business rules and the performance problem can all be wrapped up in to some very simple code. The first thing we want to do is prevent the ad-hoc addition of consultations to the patient. We still need to access the list of consultations, but we don't really have a need to modify it outside of the patient class itself. This can be done with a one-line code change to the Patient class's Consultations property:
public class Patient { private IList<Consultation> _consultations = new List<Consultation>(); public IEnumerable<Consultation> Consultations { get { return _consultations as IEnumerable; } } //ignoring other implementation details for the sake of illustrating the IEnumerable change }
Now that we have prevented the ability to do ad-hoc consultation additions, we need a way to actually add consultations. While we are doing this, we also want to enforce the business rules of the current consultation as described earlier. This is where we are going to take much of the original code that we found in the presenter / controller and place it into the patient class directly.
public class Patient { private IList<Consultation> _consultations = new List<Consultation>(); public IEnumerable<Consultation> Consultations { get { return _consultations as IEnumerable; } } public Consultation CurrentConsultation { get; private set; } public void StartConsultation() { Consultation newConsultation = new Consultation{ StartingDate = DateTime.Now }; if (CurrentConsultation != null) CurrentConsultation.EndingDate = DateTime.Now; CurrentConsultation = newConsultation; Consultations.Add(newConsultation); } }
In the end, this code shows a better encapsulation of the business rules and logic that surrounds the need to maintain a list of consultations and a current consultation. With this code in place, we could simplify the presenter / controller that we talked about initially. Rather than being forced to know all of that logic in the presenter, we can make one simple method call:
patient.StartConsultation();
With this one simple call, we have a guaranteed execution of the business rules that we need. This will allow us to recreate the ability to add a new consultation at any point in the application that we need, not just in the original presenter / controller that we were working with.
Side Note:
Part of the conversation via twitter revolved around where this type of logic should be encapsulated. From what I gathered, Colin tends to place this logic in custom collection objects, which would allow him to call patient.Consultations.Add() and still encapsulate the same business rules into that method. Like everything else in software development, there are multiple ways to solve the same problem. What does your specific situation, project, team, and business context need? Whatever your implementation needs are, though, we need to keep this type of logic and business rules enforcement well encapsulated in our systems.
Cross Posted From LostTechies.com
A coworker recently asked if we should always abstract every object into an interface in order to fulfill the Dependency Inversion Principle (DIP). The question stunned me at first, honestly. I knew in my head that this was a bad idea - abstracting into interfaces for the sake of abstraction leads down the path of needless complexity. However, I wasn't able to clearly answer his question with specific examples of when you would not want to do this, at the time. I've been thinking about this for a few days now and I think I have a good, albeit very long winded, answer. Before the question is answered, though, we need to step back and look at what DIP is all about. I've previously shown how to implement DIP and talked about why it's beneficial, so I won't be repeating that here. Rather, I want to talk about the language that describes DIP and what it really means. Robert Martin's original definition of DIP is this: A. High level modules should not depend upon low level modules. Both should depend upon abstractions.
B. Abstractions should not depend upon details. Details should depend upon abstractions. The word 'abstraction' is used three times in the definition for DIP. So, in order to understand DIP, we have to first understand some of the basics of Abstraction. Some Background On Abstraction From Wikipedia (emphasis mine): "In computer science, abstraction is a mechanism and practice to reduce and factor out details so that one can focus on a few concepts at a time" I can't say it any better than this. Abstraction can very directly lead to a system that is more understandable by helping us ignore the detail and implementation specifics, allowing us to focus on something at a higher level. This reduction in cognitive load can benefit someone that is reading the code by not forcing them to know the detail immediately. Additionally, abstraction is a form of encapsulation or information hiding, which again helps us to reduce cognitive load and produce better systems. From Wikipedia's entry on Information Hiding: "In computer science, the principle of information hiding is the hiding of design decisions in a computer program that are most likely to change, thus protecting other parts of the program from change if the design decision is changed." At the heart of abstraction and information hiding, we find the ability to change the system. The ability to change is an absolute requirement in software development and produces good design that is easier to work with, modify, and put back together as needed. The inability to change is directly called "bad design" by Robert Martin, in the DIP article. Applying Abstraction And Encapsulation To DIP So how does our knowledge of abstraction and information hiding play into DIP? First and foremost, DIP never states that we should depend on explicit interfaces. Yes, in C# we have an explicit Interface as a form of abstraction. It is a separation of the implementation detail from the publicly available methods, properties, etc, of a class. Some languages, such as C++, don't have an explicit construct for interfaces, though. From Robert Martin's original DIP article, again: "In C++ however, there is no separation between interface and implementation. Rather, in C++, the separation is between the definition of the class and the definition of its member functions." A String As An Abstraction Abstraction does always mean explicit interface constructs, as evidenced in C++. Nor does it always mean an abstract base class, which we also have available in .NET. In fact, languages such as Ruby don't really need either of these constructs. The duck-type nature of Ruby allows an implementation to be replaced at any point, without any special constructs. In .NET, though, there are a number of abstraction forms that we can rely on, explicitly. We have the obvious interfaces and base classes (abstract or not) - but we also have constructs like delegates and lambda expressions, and even the simple types that are built into the base class library. Let's look at a simple string to illustrate abstraction. As I said in my SOLID presentation at ADNUG, we can invert our dependency on database connection information. Rather than putting a connection string directly into our code that calls the database, we can use the string as our dependency and our abstraction. All we need to do is follow the basic DIP principle and provide the string as a parameter to the class that calls the database. We certainly don't need (or want, for that matter) to introduce a new interface or base class at this point. Our abstraction is simple enough to use a common type found in the .NET framework. Other Forms Of Abstraction Even if we are talking about an object, who says that the interface we are depending on has to be an explicit interface construct or base class? When I write a Domain Service that uses an Entity from my Domain, I don't create an explicit interface for that Entity. Rather, I use the Entity's inherent interface - it's public methods, properties, etc. I also use delegates on a regular basis. By specifying my abstraction as a delegate, I can further decouple the depending object from the dependant code that it needs to call. I'd be willing to bet you have used delegates as abstractions as well. Have you ever created an event handler for something like a button click? There's a delegate's abstraction at work. Abstract Judgement The point is, there is not always a need to introduce an explicit interface or base class when inverting our dependencies. We still need to apply dependency inversion and provide our implementation as a constructor parameter (or setter, though I don't like setter injection). But, that dependency doesn't have to be anything more than the interface inherent to the object, or a simple type found in .NET. You do have to be careful when making the call to not use an explicit abstraction with DIP, though. You can quickly turn your system into a ball of mud if you rely on a concrete class that is not intention revealing or well encapsulated to begin with. At the same time, too many abstractions can lead to needless complexity and make it very difficult to see the big picture of a system. Too few abstractions, though, will certainly lead to a rigid, immobile design that is hard to change. All of these problems are equally vicious - and I've been bitten by all of them in recent months. It takes good judgement calls to determine when you do and do not need an explicit abstraction for a dependency. Unfortunately, good judgement comes from experience and experience comes from bad judgement. Don't be afraid to make bad decisions - make a decision, just be sure you can reverse that decision as easily as possible.
Let's say I have a small hierarchy of object: Faults and Parts. A Fault can contain many parts, and a part has no meaning without being associated to a Fault. To ensure that I have no Parts without a parent Fault, I have this basic code in place: public class Part { private Part() { } public string Description { get; set; } } public class Fault { private IList<Part> _parts = new List<Part>(); public IList<Part> Parts { get { return _parts; } } public Part CreatePart() { Part part = new Part; _parts.Add(part); return part; } }
This ensures that I never have a Part in an "invalid" state - without an owning parent.
I also have a business rule that say that I'm not allowed to have a Part without a description. My typical implementation of this business rule has been in the UI layer - my Presenter would have the logic to require a user to enter a description when creating adding a part to a fault.
public class PartCreationPresenter { Part _part; public PartCreationPresenter(Part part) { _part = part; } public DescriptionProvided(string desc) { if (string.IsNullOrEmpty(desc)) { view.ShowDescriptionRequiredMessage(); } else { _part.Description = desc; } } }
Here's the question:
Should this "require a description" logic be in the UI layer (Presenter) the way I have it, or should I put it in the Part object and have an IsValid flag of some sort on that object?
I don't like having this coded in the presenter only. It makes the business rule very easy to break - create a part from anywhere else, and you can ignore this rule. But I'm not sure I like it in the Part object, because it would make the presenter difficult to code. How would I execute the specific view.ShowDescriptionRequiredMessage() method if the rule is coded in the Part?
Any opinions, suggestions, articles, etc. are very welcome. I'm very interested in hearing how other people are handling this situation.
Some coworkers were recently working on an object model for a simple security system. After some discussion with them, we came up with this basic model: 
A permission is defined as an activity that can be assigned to a user, or group, and can be allowed or disallowed. From a Domain Driven Design perspective, we're stating that the Permission is the aggregate root. The User object itself, while involved in this aggregate, is divorced from this aggregate's relational model - you can load and work with a User object without having to load or worry about the Permission hierarchy. The addition of the "UserGroup" is solely for the many-to-many relationship between User and Group mappings with NHibernate and is not actually part of the object model's code. Once we had this model in place, we wanted to have a simple query that allowed us to load a given permission object by activity name, for a user - whether the user was assigned directly or via a group. At a high level, this model and query should be fairly simple to work with. It turned out to be a massive learning curve in NHibernate, though. After much trial, error, and Google searching, we ended up with this NHibernate query code: ICriterion userIdMatches = Restrictions.Eq("Id", userId);ICriterion activityNameMatches = Restrictions.Eq("Name", action);ICriterion userIdAliasMatches = Restrictions.Eq("u.Id", userId); DetachedCriteria groupPermissionCriteria = DetachedCriteria.For<Permission>() .SetProjection(Projections.Property("Group")) .CreateCriteria("Group").CreateCriteria("Users").Add(userIdMatches);ICriterion groupSubquery = Subqueries.PropertyIn("Group", groupPermissionCriteria); DetachedCriteria permissionCriteria = DetachedCriteria.For<Permission>() .CreateAlias("User", "u", JoinType.LeftOuterJoin) .Add(Restrictions.Or(userIdAliasMatches, groupSubquery)); permissionCriteria.CreateCriteria("Activity").Add(activityNameMatches); ICriteria executableCriteria = permissionCriteria.GetExecutableCriteria(Session); result = executableCriteria.List<Permission>(); return result;
There were a lot of lessons learned and a lot of parts that eventually got put together. Here's a quick run-down of what we ended up with and why.
- The ICriterion's at the top of this code block are there to provide a little better readability in the real query code below.
- The groupPermissionCriteria is set up to find a permission object where the specified userId belongs to a Group that belongs to the Permission - i.e. find permissions where the user is assigned via a group. The learning curve from this perspective was the SetProjection call. Though we are not entirely sure what a projection is at this point, we did find out that it was necessary for us to set this projection so that the detached criteria could be used as a sub-query.
- The groupSubQuery is the conversion of the groupPermissionCriteria into an ICriterion so that we can do a logical Or with it in the primary query construction.
- The permissionCriteria object sets up the core criteria logic and ties together the group permission assignment with the user permission assignment.
- CreateAlias is used so that we can shorten the criteria for loading by the User assignment directly. The JoinType on the alias needs to be Left Outer Join so that we will return a proper Permission object even when there is no direct user assignment.
- After creating the alias, we can add an "Or" criterion to the query and specify that we want to match based on the user's direct assignment or a group's assignment.
- The last line of the criteria simply adds the Activity criteria to load by the activity name.
The resulting SQL will load the permission by Activity name AND (User assignment OR group assignment where the user is part of the group).
SELECT this_.PERMISSIONSID as PERMISSI1_0_3_, this_.IS_ALLOWED as IS2_0_3_, this_.ACTIVITYID as ACTIVITYID0_3_, this_.USERID as USERID0_3_, this_.GROUPID as GROUPID0_3_, activity1_.ACTIVITYID as ACTIVITYID4_0_, activity1_.ACTIVITY_NAME as ACTIVITY2_4_0_, activity1_.DESCRIPTION as DESCRIPT3_4_0_, activity1_.INACTIVE_DATE as INACTIVE4_4_0_, u2_.USERID as USERID3_1_, u2_.USER_NAME as USER2_3_1_, u2_.INACTIVE_DATE as INACTIVE3_3_1_, group6_.GROUPID as GROUPID1_2_, group6_.GROUP_NAME as GROUP2_1_2_, group6_.INACTIVE_DATE as INACTIVE3_1_2_ FROM PERMISSIONS this_ inner join ACTIVITY activity1_ on this_.ACTIVITYID=activity1_.ACTIVITYID inner join USERS u2_ on this_.USERID=u2_.USERID left outer join GROUPS group6_ on this_.GROUPID=group6_.GROUPID WHERE activity1_.ACTIVITY_NAME = :p1 and ( u2_.USERID = :p2 or this_.GROUPID = ( SELECT this_0_.GROUPID as y0_ FROM PERMISSIONS this_0_ inner join GROUPS group1_ on this_0_.GROUPID=group1_.GROUPID inner join USERS_GROUPS users5_ on group1_.GROUPID=users5_.GROUPID inner join USERS user2_ on users5_.USERID=user2_.USERID WHERE user2_.USERID = :p3 ) )
Using a sub-query to load based on the group is not the most optimal way of loading the permission for the group assignment. However, since all of the joins in the main query and the sub-query are done on primary and foreign keys in the tables, performance should not be an issue. The only real performance concern for this query is the activity name in the where statement. A simple unique constraint and index on the activity name, though, will solve that problem.
Earlier this week, I was doing a training session with my current team, building a sample Org Chart application. One of the user stories we were working with, talked about assigning managers to department and had the following acceptance criteria: - When assigning a manager to a department, ensure that they are an employee of that department, and remove them from their old department
The basic code that we came up with worked well and was encapsulated into a service object in the domain. public class AssignmentService { private IDepartmentRepository _repository; public AssignmentService(IDepartmentRepository repository) { _repository = repository; } public void AssignManagerToDepartment(Employee manager, Department assignedDepartment) { Department previouslyAssignedDepartment = _repository.GetDepartmentForEmployee(employee); if (previouslyAssignedDepartment != assignedDepartment) { previouslyAssignedDepartment.RemoveEmployee(manager); _repository.Save(previouslyAssignedDepartment); } assignedDepartment.AssignEmployee(manager); assignedDepartment.AssignManager(manager); _repository.Save(assignedDepartment); } }
Problems
At first glance, this code seems simple enough, but we quickly ran into a problem when we started coding for another story and acceptance criteria.
- When creating a department, allow a manager to be assigned
Off-hand, this seems like a very simple situation. We'll just call the AssignmentService object after we create the Department. However, there's a large potential problem with this - the AssignmentService handles the entity persistence for both of the departments that were changed, when the manager is assigned. With that in mind, the developer working on the new requirement would have to ensure that the department is created with all of it's required fields and information before calling this service object. If the developer allows the manager to be assigned before the department's info has been completely filled in, we may accidentally allow an invalid state for a department object when calling the AssignmentService. Since the AssignmentService tries to save the department being assigned, we may also have created an semantic coupling between the department creation process and the AssignmentService - the developer needs to know that the AssignmentService will save the department. If they don't know this, they may try to save the same entities more than once, may try to save an invalid state, etc, etc, etc.
Unfortunately, we didn't have time to fix the problem during the training session - we had to stop the training almost immediately after we discovered this issue. I've got a few ideas running around in my head, on how to fix this, but I'm not 100% sure that I like any of them.
Possible Solutions
First and foremost, though, it's becoming rather apparent to me that we want to avoid entity persistence calls from within the domain service objects. This essentially means that the AssignmentService object would have to return the assigned department, and the 'previously assigned department' objects, so that the coordinating presenter could save them both. That seems a little odd, if you ask me.
One of my coworkers suggested that we allow the service to save the previously assigned department, since that is an self-contained operation in the method (the method loads the department, modifies it, and saves it) and then return the assigned department so that the presenter can save it. This option seems a bit better off-hand, but I'm still worried about the possible side effects of the AssignmentService saving the previously assigned department.
Questions
What are your thoughts on this situation... How would you solve the dilemmas that I'm pointing out? Is there a better design for the model that would eliminate this problem and make the department persistence more obvious, without having to 'know' when it is persisted?
Is the idea of not allowing entity persistence from a domain service object valid? Should we go with allowing the self-contained operations to persist the entity in that operation?
Or ???
During a TDD/BDD training session with my team, today, we wrote the following code: public void SaveRequested() { Employee employee = Department.CreateEmployee(firstName, lastName, email); if (employee == null) repository.Save(Department); }
I ignored this when we first wrote it, as it was "technically correct", but later returned to this code as a point of refactoring for greater insight and expressiveness. When we came back to it, though, I started off by asking a series of questions:
- Why would that method return a null?
- Why would it not return a null?
- Why is it important to not call repository.Save if the employee is null?
The code is essentially a mystery to anyone that hasn't memorized the details of the Department.CreateEmployee method. Worse yet, I can make a lot of assumptions about this code - but every one of them is going to be wrong until I can prove that they are right by reading the code in Department.CreateEmployee.
So how do we refactor this code for better insight and more expressiveness? How do we change this code so that the assumptions a reader creates in their own mind are more likely to be correct - or better yet, are irrelevant because you don't need to even care about the details of the Department.CreateEmployee method? First and foremost, you need to understand what this code is supposed to be expressing - what it's supposed to be doing and why. If you don't know, you need to ask the person that wrote it. If they don't know (or aren't around), you need to find someone that can help you determine the intended behavior. Ultimately, you may need to dig into the Department.CreateEmployee method to figure it out for yourself.
In this situation, we were trying to express some business rules from a set of acceptance criteria on a user story:
- When creating an employee, the employee must belong to a department
- When creating an employee, require a first name, last name, and email address
This code is a factory method on the Department object that is used to ensure these rules are satisfied. In this case, if CreateEmployee returns a null, one of these rules was violated and we should not attempt to save the Department (with the associated employee, or lack there-of). If CreateEmployee returned an employee, we satisfied the rules and we can save the Department and it's associated employees.
The code that we have in place doesn't express any of that intent, though. What we really want is code that tells us that the employee creation failed - and probably why it failed. Fortunately, the changes can be extraordinarily simple.
public void SaveRequested() { EmploymentResult employmentResult = Department.Employ(firstName, lastName, email); if (employmentResult.Success) { repository.Save(Department); } else { view.DisplayError(employmentResult.FailureReason); } }
UPDATE: Thanks to Martin Linkhorst for the comments, and making this code even more expressive (changing from Department.CreateEmployee to Department.Employ)
By creating an AddEmployeeResult object and supplying a few additional bits of information in it, our code became much more expressive of it's intent. We can now read the SaveRequested method and have a much better chance of knowing what it is really doing - try to create an employee and if it successfully created, then save it. Otherwise, display the failure reason.
What assumptions does your code create in the minds of the readers? Are these assumptions correct, incorrect, or unintelligible? And finally, what can you do to make your code assumption-free and/or assumption-irrelevant?
Inspired by my recent readings in Domain Driven Design - specifically Chapter 10, "Supple Design" - and recent posts by David Laribee and Nigel Sampson, in combination with the recent pains I've been putting myself through, trying to test query generation code in a search screen, I decided to spike out a quick example of a reusable Specification implementation. Rather than repeat what's already been said, I'm just going to get straight to the code. using System; namespace Spec_Spike { class Program { static void Main() { Foo foo1 = new Foo(); foo1.Bar = "Test"; ISpecification<Foo> equalSpec = new Specification<Foo>(foo => foo.Bar == "Test"); ISpecification<Foo> notEqualSpec = new Specification<Foo>(foo => foo.Bar != "Not Equal To This Text"); ISpecification<Foo> falseSpec = new Specification<Foo>(foo => false); ISpecification<Foo> passingSpec = equalSpec.And(notEqualSpec); ISpecification<Foo> failingSpec = passingSpec.And(falseSpec); Console.WriteLine(equalSpec.IsSatisfiedBy(foo1)); Console.WriteLine(notEqualSpec.IsSatisfiedBy(foo1)); Console.WriteLine(passingSpec.IsSatisfiedBy(foo1)); Console.WriteLine(failingSpec.IsSatisfiedBy(foo1)); } } public class Foo { public string Bar; } public interface ISpecification<t> { bool IsSatisfiedBy(t obj); ISpecification<t> And(ISpecification<t> lhs); } public class Specification<t>: ISpecification<t> { private readonly Predicate<t> _pred; public Specification(Predicate<t> pred) { _pred = pred; } protected Specification(){} public virtual bool IsSatisfiedBy(t obj) { return _pred(obj); } public ISpecification<t> And(ISpecification<t> andSpec) { return new AndSpecification<t>(this, andSpec); } } public class AndSpecification<t>: Specification<t> { private readonly ISpecification<t> _spec1; private readonly ISpecification<t> _spec; public AndSpecification(ISpecification<t> spec1, ISpecification<t> spec) { _spec1 = spec1; _spec = spec; } public override bool IsSatisfiedBy(t obj) { return (_spec.IsSatisfiedBy(obj) && _spec1.IsSatisfiedBy(obj)); } } }
Drop this code into a console app in C# 3.5 and watch the magic happen. Here's the output:
These are the results that I expected - the first 2 individual specs passed, the first combined spec passed, and the last combined spec failed.
Overall, I'm fairly excited about the possibilities here. I'm thinking that I may actually be able to properly unit test the query generating code in my search screen with this basic technique. I'm still not 100% sure on that, but I plan on trying, anyway.
For more information on the Specification pattern, I highly recommend you read the previously linked posts by David Laribee and Nigel Sampson, in addition to reading all of the Domain Driven Design Book. This is one of those books that should fundamentally change the way you think about software development.
Myself and 10 other developers in my company went through a day of BDD / TDD training, with Scott Bellware, yesterday. It was a lot of fun, very challenging at times, and covered a lot of topics including an overview of Agile and Behavior Driven Development, all the way down to writing Specification Tests, doing Test Driven Development and refactoring the model to improve readability, maintainability, flexibility, etc. I took notes via index cards (love that cool-aid) and wanted to share. I don't expect these notes to make sense to everyone. Hopefully it will spark some dialog in someone's mind and cause them to dig further. First off - the quote of the day. "Can I be honest with you and say that I've been wanting to touch your keyboard, all day?" Now for my notes. User Stories [Role], [Goal], [Motiviation] - As a [role], I want to [goal], so that [motivation]
- Example: As a nurse, I want to record a patients vital signs, so that I can determine their medication and care needs
- Motivation is critical - it determines how the development team understands and implements the story. It determines the user experience, how things are integrated, how the software is designed, etc.
Acceptance Criteria - Acceptance Criteria is used to drive code, not the story, directly
- may change at any point, up to implementation
- is used to drive code design, test design, implementations, etc.
- should be spoken in domain language
- may include non-functional, technical details such as database tables, infrastructure, performance, etc
- All acceptance criteria must be met and tested / verified before a story is considered done
Specification Tests - Test Fixture per Class is an anti-pattern (on a personal note, this problem bothered me for months before I discovered BDD)
- Context Specification or Behavior Specification testing
- When [verb] then [verb]
- "When [verb]" is the context
- "Then [verb]" is an observation of the behavior
- Based on Acceptance Criteria, but not "code-gen'd" from acceptance criteria
Story Estimation - Agile Poker: uses generalized Fibonacci sequence as order of complexity
- "?", 0, 1/2, 1, 2, 3, 5, 8, 13, 20, 40, 100, infinite
- everyone throws their estimate at same time
- if estimates have significant outliers, discussion occurs to understand why, get more detail, etc. and re-throw may happen
Entity Data vs. Aggregate Data - Entities should never contain aggregate data
- Aggregate data is for reporting and other aggregate needs
- If you need aggregate data to process something, write an SQL query, stored proc, etc. - don't use an ORM like NHibernate
- We don't want a "Customer" entity to need 10,000 "Order" entities, to aggregate data for processing; write a query to aggregate instead
- We don't want to persist data that can be calculated / aggregated, generally (performance issues may override this)
Domain Services - Can have dependencies on external systems
- are part of domain logic, therefore are in domain model / assembly
- are "Doers" of process that don't fit into entity and entity logic, directly
- coordination of entity logic
- can include calls to data access, logging, etc.
Continuous Integration - Not just continuous compilation of code
- Full end to end integration of all code, components, databases, services, etc
- Full suite of integration testing including database testing
- Do not allow commits if build is currently broken
- do not allow defects to live - fix immediately, to fix build
- "Defect" is broken software, "Bug" is functional but wrong
Daily Scrum - 3 Questions everyone answers:
- What did I do yesterday?
- What am I doing today?
- What issues am I having?
- Each person should answer quickly - 1 or 2 minutes, max
- further discussion happens outside of the Scrum meeting
Productivity of Dev Team - RAD and other non-review, non-iterative based management causes problems and loss of productivity
- we need constant review of the design to ensure good design
- shorten the feedback loop and get constant review of the design, to always improve the design, via pair programming, work cells, retrospectives, etc.
- good design will cause productivity gains in the development team beyond the capabilities of any tools
Whiteboard Diagramming vs. Details Specs - White board diagramming and human interaction is always better than detailed documents and specs in UML
- Human interaction leads to knowledge crunching and learning, not just reading a repeating
- Take pictures, don't re-draw in UML; don't waste time with it
- Video the entire conversation is even better, so others can learn from the knowledge crunching that occurs; capture the human interaction, body language, etc.
Recently, my development team has started up our Friday Lunch-n-Lean sessions where we all read a chapter of a given book and discuss that chapter amongst the group during lunch on Fridays. We're now on Chapter 2 of Domain Driven Design, by Eric Evans. When I first started reading DDD around a year and a half ago, I honestly thought the first 4 or 5 chapters were wordy, boring and repetitious. Going back to the beginning of the book with my experience and knowledge from the last year and a half, I can't believe that I thought this. The first two chapters alone have been completely mind-blowing for me, on my second read of the book. I don't know if it's the combination of other knowledge, the experience I've had trying to implement DDD's concepts, or what... At this point, I can hardly turn a page without having 10 or 15 highlighted phrases and sentences. Seriously - there's rarely a page in the first two chapters that does not have something highlighted. If your involved in any part of software development - management, analysis, development, documentation, or anything else - and you've never read this book; you need to read at least the first 3 chapters at least 3 times, if not the entire book.
|