In addition to our 'Code Review Challenge' that I discussed recently, we have been using another retrospective game - 'Name That Standard'.

The idea is fairly simple. At the beginning of any retrospective, we go around the room and ask each team member to list out a standard that we are using in our project. This can be any standard that anyone feels the team is using or needs to be using, including coding conventions, architectural patterns, business and project management processes, communication means, etc. The intention is similar to the intent of 'The Code Review Challenge' - socialization of the the system. Only in this case, we are applying the term 'the system' at a much higher level - our organizational and project management / implementation system as a whole.

Some thoughts on running 'Name That Standard':

• Require not only naming of the standard, but accurate description of the standard and an example of where it is used. Alternatively, you may want to list a brief description out on a projector or whiteboard and see who can name the standard being described.
  
• Encourage the team to think about the team as a whole, not just their area of responsibility.
  
• Encourage the team to list standards that they don't understand, so that the rest of the team can chime in with assistance and help the team member in question grow
  
• Don't just let people yell out standards. go around the room, one person at a time. we got a little chaotic at first, and it was hard to know who said what.
  
• Encourage people to ask questions like 'i thought we were using this standard, but i saw someone else doing that standard'
  
• Encourage the standards to become more and more low level and detailed, over time (over multiple iterations)
  
• Encourage the standards to evolve and change for the better, over time (over multiple iterations)

In our implementations of this game, we've managed to get a fairly good list of standards written down and also resolved some questions on what standards we actually are using. For example, we had 3 dynamic mocking frameworks in our code base - Rhino Mocks, Moq, and a home grown one that we lovingly called 'BrandoMocks' (named for our team member, Brandon, who wrote it for fun one night). The end result of that discussion was to throw out Moq and BrandoMocks, adopting Rhino Mocks as our standard for this project.

Having done 'Name That Standard' in two consecutive iteration retrospectives, I think I can say that it has been very successful. The team as a whole is becoming more and more aware of the standards that we are using, and the individual members are beginning to contribute more and more to the standards.

With each passing iteration, and each new technique for creating collective ownership and socializing the system, I see the team coming together and really forming a team vs. a bunch of individual developers working on the same system.

My team tried out 'The Code Review Challenge' in today's retrospective. Overall, I think it went pretty well for the first time that we've tried this. Here's some initial thoughts after our first attempt.

1. Make sure the team knows what's going on before you start. I was not leading the review, and I had to step out right when it started. I believe the team lead explained the idea, but I'm not sure I had fully explained to her. In the end, ensuring that each team member is comfortable with the idea and the process is important.
2. Encourage discussion with the group during the review. Don't force the talk from the reviewer, but ask questions that will help lead them to conclusions and descriptions of what is going on.
3. Don't expect everyone on the team to be able to recognize the same standards, practices, or issues. each team member will have a different understanding of the standards that we are trying to apply, at any given time. encourage learning help others to see what you see.
4. make sure everyone on the team has a chance to review someone else's code. i think we ended up with a good mix of people reviewing and being reviewed, but it wasn't planned, it just kind of happened. next time around, we're going to ensure that it happens by planning ahead based on the work done by the people in the review.

i think there was a lot of benefit in doing the reviews in this manner. we ended up with a lot of good comments, suggested improvements, etc. i'm happy to say that the code review challenge was a success and that we'll continue to do it in future retrospectives and reviews.

A Food Network Challenge

During this season's 'The Next Food Network Star', there was an episode where the contestants had to cook a dish and review it in front of a camera, describing it's visuals and taste. The twist to the challenge was that they were assigned someone else's dish to review, after everyone had already cooked. Once the assignments were made for who was going to describe who's dish, they were not allowed to see the dishes anymore. A given contestant was then put in front of a camera and had their assigned dish uncovered in front of them. From that point, they had 90 seconds to describe the dish that was in front of them - that included figuring out what it was, what it looks like, what it tasted like and what it smelled like. This was a rather tough challenge to watch, and a few of them didn't do so well.

A Code Review Challenge

What if we take the Next Food Network Star's challenge and apply it to code reviews? When we sit down in a code review session, rather than having the person who wrote the code doing the driving and telling us what is going on, have someone who has not seen the code do the driving - read the code, read the unit tests, etc - and describe to us what is going on. 90 seconds wouldn't be nearly enough time, though, so we'd have to change that time limit based on the size and complexity of the functionality being reviewed. Other than that, though, it seems like it would be a great way to test the team's ability to pick up an area of the system that they have not seen before and quickly learn it so that they can work in it.

Standardized Work - Judging Code And Code Reviewer

There are 2 real benefits to the code review challenge, as I see it:

1. Judging how well did the coder apply our coding and development standards
2. Judging how well the reviewer can recognize and describe the standards being applied

We are now building our system from User Stories, with Context / Specification style unit testing. One of the intents of this style of unit testing is to provide an easy-to-read and easy-to-learn set of examples for developers that need to know how to do something or how something works in the system. If the unit tests are properly written and expressive of the code's intent - it should be really easy to see what's going on. If we have good standards in place, a developer should be able to find the code in well organized areas, easily, and know where to look for the various components and parts.

It seems to me that the code review challenge would help to prove a number of these key factors in our projects and our teams:

• Coding and architecture standards
  • How well the standards are known
  • How well the team follows the standards
• Expressive Code
  • How easy is it to read the code
  • How easy is it to understand the code's intent
• Maintainable Code
  • How easily can the code be changed and extended
  • How quickly the team can socialize and collectively own the system

A Challenge In Name, A Learning Experience In Practice

I would avoid applying any sort of point system or other reward / incentive for the code reviews. It seems that this could easily backfire and cause junior team members (in skill or time on the team/project) to feel intimidated or feel that it's an unfair process comparing them to the senior level team members. The intention of the code review challenge is not to cause dismay in any team member, but to create a learning environment by continuously challenging every member of the team. Ultimately, the code review challenge should foster the socialization of the system and lead to a strengthened sense of collective ownership, resulting in a better system.

Nothing New Under The Sun?

With all that being said - I'm doubtful that I'm the first person to ever hold a code review like this. I'd be interested in knowing if there's any standard development practices (Agile or otherwise) that are heading in the same direction. Any advice or opinions of you, the reader, would also be very welcomed.

If you know of a resource that describes the same basic ideas or if you have any input to help me improve the idea, please leave a comment and let me know.

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.

Test Driven Development Is To Unit Testing As Interaction Design (IxD) Is To Accidental Design

One of the major problems with writing unit tests after the code is that it is very natural to write tests that prove the code works the way it was written, instead of the way it should work. Writing code test-first (via whatever flavor of Test Driven Development) flips that scenario right side up. TDD helps to ensure that code is written to work according to specifications and not the other way around.

Similarly, designing and/or implementing a UI after a behavior or process has been coded is a likely to result in a UI that fits the code model and not a model that fits the needed interaction and workflow. This situation must also be flipped right side up - interaction design should be done before, or at least in parallel, with coding. It cannot be left to accidental or incidental happenstance - interaction design must occur with proper interaction patterns and practices in mind.

Overlapping IxD and TDD

To overlap interaction design and test driven development, there are a few key words that need to be borrowed from interaction design. Fortunately, they easily fit within TDD development techniques and philosophies.

Epistemic work is exploratory in nature, or a process of trial and error through research. TDD and interaction design sketching are both epistemic. TDD explores API possibilities and allows easy trial and error to find the simplest implementation for what you need at the time. UI design sketches also allow you to quickly explore interaction designs - whether it's a white board, pencil and paper, graphics design software, or even quick-hack forms layout in IDEs. You can quickly and easily throw away a bad API in TDD and you can quickly and easily throw away a bad UI/Interaction design when you have nothing more than pencil sketches or white board drawings.

Pragmatic work is very structured and step-by-step in it's nature, implementing patterns and practices to fulfill what is needed at the moment. Implementing Code after writing unit test and implementing UI after designing around constraints are both pragmatic. TDD is pragmatic in that you only implement what is needed to properly pass the tests that have already been written. Similarly, with previously designed interactions and UI elements, implementation can be easily limited to what is needed for the UI.

With epistemic and pragmatic work covering both interaction design and test driven development, it seems that they are a natural pair. An analysis of a user story and it's acceptance criteria will create the unit tests that we need. At the same time, the same analysis can be applied to interaction designs. Additionally, a strong understanding of how a UI will look can have a profound impact on the code that is written, and vice-versa. Therefore, it is natural that interaction design and test driven development are done at just-in-time intervals - before the real work is implemented in the real code and UI platform.

IxD As Part of "Done"

Despite interaction design being a required part of UI development, not all user stories require a UI. Interaction design may not fit into a swim lane board or be part of every story's "done" criteria. However, interaction design will always be done for any story that does have a UI - it may simply be an accidental or incidental part of the software development process for a team, though. If it's safe to assume that the work will be done, then it is the team's responsibility to ensure that it is done correctly. Don't let interaction design happen accidentally or incidentally in the development process. Set a standard of always including interaction design in the development process, the same way Model View Presenter/Controller is a part of development.

Collective Ownership

One of the principles that I’ve tried to instill in my team(s) is that we are a collective ownership team. I've talked about Collective Code Ownership before, and I still believe it is needed to have a solid team. My views have recently expanded a little to include more than just the software developers on the project team - crossing all functional boundaries in a given project team.

Collective Ownership means that there is no one person on the team – developer, ba, tester, etc. – that "owns", controls, or is solely responsible for any one portion of the system. Code, documentation, and all other artifacts are owned by the entire team, as we should be working as a team to complete the project.  Obviously there are people who specialize in one area or another, so they will have more influence in those areas. Depending on the corporate culture that you work in, there may also be some boundaries that we can’t cross due to politics, etc. At the very least, though, every last line of source code in a project is owned by the team and is the responsibility of the team. The moment any artifact (code, images, documents, etc) is committed to source control or any other common project storage, the person that wrote/created/modified it is no longer directly responsible for it – the entire team is, and anyone on the team that sees a problem is expected to fix it or call attention to it so it can be fixed.

This principle is key to the team being able to create great software. We don’t want people to say "my code is better, and my area of the application works, but your code is bad and doesn’t work". This kind of divisive thinking will create a poorly constructed system that doesn’t fit together very well. We need to ensure that the entire team is saying "our code is good", or "our code needs work", and "I can help improve the team’s code base".

Your Code Is Terrible Because My Code Is Terrible

I’ve heard several times that a code base is only as good as the weakest developer on the team. I believe this is true – especially in a collective ownership environment. The moment a developer commits code, it is available for any other team member to modify, and therefore the quality of the code is immediately suspect and subject to the standards of the weakest team member.  Despite the potential problems that collective ownership creates, its very nature should help to bring the skill level of every developer on the team up, alleviating the 'weakest code' problem.

When a 'strong' developer sees the code of a 'weak' developer (those terms being relative to a lot of factors, of course) the strong developer should instinctively want to help the weak developer so that they are both producing better code. When a weak developer sees code that they believe is strong, they should instinctively want to mimic the qualities of that code and either ask for assistance or work to understand how to produce it on their own. Though it does take time and a lot of hard work, the end result should be that the collective weakness of the team disappears. The 'weakest' developer on the team will become an unknown – even irrelevant. Everyone will have strengths and weaknesses, but the team will balance each other out and the code that is delivered will be good.

Socializing The Design

For collective ownership to work, though, we have to ensure that the entire team is comfortable with the entire system. Each team member must be able to understand, update, and "own" any part of the system that they are currently working on (ownership being transitive, and forfeit once they commit the change, of course). For an entire team to be comfortable with the entire system, we need to socialize the design - that is, ensure the team understands the architecture and design fundamentals through the social aspects of working on a team. When we do a good job of socializing the design, it becomes much more natural for the team to collectively own the code. A given developer may not know anything about a given area of the system, but because there are good standards in place and everyone is aware of the overall design of the system, everyone on the team should be able to dig into any given area and learn it quickly.

The real challenge in socializing a design is figuring out what works for your team. There are as many ways of socializing the design, as there are systems and teams - some of which include:

• Code reviews (formal and informal)
• Pair programming
• Email, Presentations, and other “here’s how you do xyz”
• Wiki's, Forums, and other ways of asking “what do you think about xyz or 123”
• Iteration retrospectives
• Etc, etc, etc

No two teams or situations are alike, either. If you are limiting yourself to just a few of these techniques (or only the techniques listed here), you may not reach the full potential of your team's understanding. Try different combinations of socialization techniques and see what works well for your team.

End Goal: Continuous Improvement

In the end, a socialized design and collective ownership enable teams to work more efficiently by preventing the bottlenecks that 'I maintain XYZ' will create.  Code quality will rise, the team work work together, and we can start to move forward into better standards, better practices, and continuous improvement.

My team's first swim lanes!

From left to right: Backlog, In Dev, In Test, Docs, Done.

sweeeeet.

In the last month or two, I have been hand coding a lot of mock and stub objects and it has become a nightmare to manage. My primary reason for doing this by hand was that Rhino Mocks 3.4 and older did not fit with the BDD style unit tests that I was writing. Yes, I made it work in a few places, but it was ugly and annoying.

Fortunately, Ayende has cleaned it all up with the new syntax and made it very easy to assert that individual expectations were met, with v3.5. I finally got around to trying out the new syntax today, and I immediately fell in love with it. For example:

[Concern("User Administration")]

public class When_accessing_the_system_as_a_non_administrator : ContextSpecification

{

    private IMainView view;

    private IMainView GetView()

    {

        IMainView mockView = MockRepository.GenerateMock<IMainView>();

        mockView.Expect(v => v.EnableUserManagement()).Repeat.Never();

        mockView.Expect(v => v.DisableUserManagement()).Repeat.Once();

        return mockView;

    }

    protected override void Context()

    {

        User administrator = new User();

        administrator.IsAdministrator = false;

        CurrentUser.User = administrator;

        view = GetView();

        new MainPresenter(view);

    }

    [Observation]

    public void Should_not_display_the_User_Management_option()

    {

        view.AssertWasNotCalled(v => v.EnableUserManagement());

    }

    [Observation]

    public void Should_hide_the_user_management_option()

    {

        view.AssertWasCalled(v => v.DisableUserManagement());

    }

}

In the "GetView" method, I am setting up two very distinct expectations on my mock object.

1. Never Call the EnableUserManagement method.
2. Call the DisableUserManagement method once.

With the new Rhino Mocks syntax, I can easily verify that each one of these expectations was called via my "should" observations.

The "AssertWasNotCalled" extension method verifies that an expectation of Repeat.Never was setup and that the method was not called.

mockView.AssertWasNotCalled(v => v.EnableUserManagement());

And the "AssertWasCalled" extension method verifies that the expectation was to call the method, and that the method actually was called.

mockView.AssertWasCalled(v => v.DisableUserManagement());

I like it. The new syntax has really simplified my Context Specification testing.

Scott Bellware (who finally started blogging again), is posting a series on Sustaining Capacity in Maturing Agile Software Teams. It's worth a read, and may be familiar territory to anyone who has done Agile and been reading about Lean.

In the 3rd Post - Recognizing Entropy - he talks specifically about synchronizing the development testing and QA testing efforts:

Development and QA Test Synchronization. Without greater synchronization between development and QA testing, valuable input often comes too late in a development or release cycle to be effective. Test design and test architecture are valuable inputs to development, and software design and architecture are valuable inputs to QA testing. Teams often loose unrealized capacity by not pursuing means to do more development and testing in parallel.

This one really hit home for me, and is a shining beacon to the problems that my current team is going to face, soon. We had our first iteration planning and kickoff meeting yesterday, and the whole team was involved - except the testers from the QA department. For reasons that I have no control over (and honestly - they are legitimate business reasons), the QA department is not coming on to the project for another 2 months (at least).  Today, the team leadership was discussing how the QA personnel would create their test scenarios, and I brought up the point that creating the test scenarios after-the-fact (after the iterations are done) is going to be very problematic.

If a dev team is working from User Stories, and those User Stories are divided along various aspects of behavior, it is very likely that a single screen, workflow, or 'function' of a system is going to be divided into multiple user stories. When the QA team is involved with testing in parallel with the development team, this is not an issue because the QA team will be updating their test scenarios in each iteration. This allows them to keep the scenarios for a given screen, workflow, or 'function' of the system, growing over time. However, since our QA team is going to come into the project at least 2 months after the development team has started coding and testing, the QA team would have to sift through all of the current and previous iteration stories, try and aggregate them down into the screen, workflow, or 'function' of the system, and build test scenarios from there. If there is even a modest number of stories to sift through, this would be a daunting task for the QA team - they would be in a 'catch-up' mode from the beginning of their QA cycle, likely through the end of the project.

As a result of the QA team being unable to join our project immediately, we are forced to modify our entire development process and account for the needs of the QA team, after the fact. Essentially, we are creating additional swim-lanes for our stories to travel through, during an iteration:

1. Backlog
2. In Development
3. Developer Testing
4. Ad-Hoc Testing (BA's, Developers, etc. testing the app)
5. QA Functional Spec Documentation
6. Done

Swim-lane #4 and #5 are directly affected by the QA team not being able to enter the project immediately, and are causing the rest of the team to lower it's capacity. We (developers and BA's) are now required to do our own ad-hoc testing during the iterations, instead of letting the QA team take care of that; and we (the developers and BA's) are now required to create functional specifications, based on the user stories that are completed, to hand to the QA team once they are on the project. The functional specification documents will need to be updated for any given User Story, before the ticket is considered complete (the functional spec updates have become an acceptance criteria on every User Story).

End result - because we cannot get the QA team on our project immediately, and ensure that they are testing in parallel to the developers, we as the development team are losing capacity and building entropy from the very first day of the very first iteration.

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 ???