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.