views:

122

answers:

3

Hi all,

I currently have service classes that look something like this

public class UserService : IUserService 
{
    private IAssignmentService _assignmentService;
    private ILocationService _locationService;
    private IUserDal _userDal;
    private IValidationDictionary _validationDictionary;
    public UserService(IAssignmentService assignmentService, ILocationService locationService, IValidationDictionary validationDictionary)
    {
        this._assignmentService = assignmentService;
        this._locationService = locationService;
        this._userDAL = new UserDal();
        this._validationDictionary = validationDictionary;
    }

    .....

    private void ValidateUser(IUser user)
    {
       if (_locationService.GetBy(user.Location.Id) == null)
          _validationDictionary.AddError("....");
       if (_assignmentService.GetBy(user.Assignment.Id) == null)
          _validationDictionary.AddError("....");
       .....
    }
}

And DAL classes that looks like this

public class UserDal: IUserDal
{
    private IAssignmentDal _assignmentDal;
    private ILocationDAL _locationDAL

    public UserDal()
    {
        this_assignmentDal = new AssignmentDal();
        this._locationDal = new LocationDal();
    }

    public int AddUser(IUser user)
    {
       // db call and insert user
       _locationDal.Add(user.Location);
       _assignmentDal.Add(user.Assignment);
    }

    public IUser GetUser(int id)
    {
       ..DB Call

       IUser user = new User() { userData, GetLocation(dr["Location_Id"]),GetAssignment([dr["Assignment_Id"]);
       return user
    }

    private ILocation GetLocation(int id)
    {
        return new LocationDal().GetById(id);
    }
    private IAssignment GetAssignment(int id)
    {
        return new AssignmentDal().GetById(id);
    }
}

I was wondering if it was considered bad design to have the Service layer talk to other Service Layer Objects and Dal talks to other Dal Objects?

Thanks in advance

A: 

I dont know what you mean by Dal, but generally , you need to balance the level of cohesion and coupling in your application.

In this case, i would question why an instance of a UserService can validate other IUsers. That seems to be like coupling which might be problematic. However, I dont know your application, so i might be mistaken.

Draw out a class diagram to make it easy to see the coupling among the classes.

Andrew Keith
DAL- Data Access Layer
Abhijeet Patel
A: 

You could follow a model similar to the "DataContext" model followed by LINQ2SQL or Entityframework i.e have a Context which keeps track of entities such as "User" etc. Your service layer in turn just talks to the Context to either query across entities or perform operations across entities if needs be. This way your individual entities such as the "User" entity will not need to have direct knowledge(coupling) to other "unrelated" entities and the Context exposes the entities to the client.

Abhijeet Patel
+5  A: 

Given the design of your examples, you are going to run into what I like to call dependency hell. It is certainly an option to go down the route you are taking, but it will lead to a highly coupled architecture that will likely be very difficult to maintain and refactor. If you abstract a little bit more, however, you can simplify your architecture, organize responsibilities a bit more, and separate concerns in such a way that managing your dependencies will be a lot easier.

The UserService, AssignmentService, and LocationService seem like CRUD style services. A more appropriate term for them would be Entity Services. An entity service should be solely responsible for CRUD operations of the immediate entity, and nothing else. Operations that involve multiple entities, entity relationships, etc. can be pushed to a higher-level service that can orchestrate larger-scale operations. These are often called Orchestration or Task Services.

I would recommend an approach like the following. The goals here are to simplify each service to give it the smallest responsibility scope, and control dependencies. Simplify your service contracts to reduce the responsibilities of your existing entity services, and add two new services:

// User Management Orchestration Service
interface IUserManagementService
{
    User CreateUser();
}

// User Entity Service
interface IUserService
{
    User GetByKey(int key);
    User Insert(User user);
    User Update(User user);
    void Delete(User user);
}

// User Association Service
interface IUserAssociationService
{
    Association FindByUser(User user);
    Location FindByUser(User user);
    void AssociateWithLocation(User user, Location location);
    void AssociateWithAssignment(User user, Assignment assignment);
}

// Assignment Entity Service
interface IAssignmentService
{
    Assignment GetByKey(int key);
    // ... other CRUD operations ...
}

// Location Entity Service
interface ILocationService
{
    Location GetByKey(int key);
    // ... other CRUD operations ...
}

The process of creating a user and associating it with a location and assignment would belong to the UserManagementService, which would compose the lower-level entity services:

class UserManagementService: IUserManagementService
{
    public UserManagementService(IUserService userService, IUserAssociationService userAssociationService, IAssignmentService assignmentService, ILocationService locationService)
    {
        m_userService = userService;
        m_userAssociationService = userAssociationService;
        m_assignmentService = assignmentService;
        m_locationService = locationService;
    }

    IUserService m_userService;
    IUserAssociationService m_userAssociationService;
    IAssignmentService m_assignmentService;
    ILocationService m_locationService;

    User CreateUser(string name, {other user data}, assignmentID, {assignment data}, locationID, {location data})
    {
        User user = null;
        using (TransactionScope transaction = new TransactionScope())
        {
            var assignment = m_assignmentService.GetByKey(assignmentID);
            if (assignment == null)
            {
                assignment = new Assignment { // ... };
                assignment = m_assignmentService.Insert(assignment);
            }

            var location = m_locationService.GetByKey(locationID);
            if (location == null)
            {
                location = new Location { // ... };
                location = m_locationService.Insert(location);
            }

            user = new User
            {
                Name = name,
                // ...
            };
            user = m_userService.Insert(user);
            m_userAssociationService.AssociateWithAssignment(user, assignment);
            m_userAssociationService.AssociateWithLocation(user, location);
        }

        return user;
    }
}

class UserService: IUserService
{
    public UserService(IUserDal userDal)
    {
        m_userDal = userDal;
    }

    IUserDal m_userDal;

    public User GetByKey(int id)
    {
        if (id < 1) throw new ArgumentException("The User ID is invalid.");

        User user = null;
        using (var reader = m_userDal.GetByID(id))
        {
            if (reader.Read())
            {
                user = new User
                {
                    UserID = reader.GetInt32(reader.GerOrdinal("id")),
                    Name = reader.GetString(reader.GetOrdinal("name")),
                    // ...
                }
            }
        }

        return user;
    }

    public User Insert(User user)
    {
        if (user == null) throw new ArgumentNullException("user");
        user.ID = m_userDal.AddUser(user);
        return user;
    }

    public User Update(User user)
    {
        if (user == null) throw new ArgumentNullException("user");
        m_userDal.Update(user);
        return user;
    }

    public void Delete(User user)
    {
        if (user == null) throw new ArgumentNullException("user");
        m_userDal.Delete(user);
    }
}

class UserAssociationService: IUserAssociationService
{
    public UserAssociationService(IUserDal userDal, IAssignmentDal assignmentDal, ILocationDal locationDal)
    {
        m_userDal = userDal;
        m_assignmentDal = assignmentDal;
        m_locationDal = locationDal;
    }

    IUserDal m_userDal;
    IAssignmentDal m_assignmentDal;
    ILocationDal m_locationDal;

    public Association FindByUser(User user)
    {
        if (user == null) throw new ArgumentNullException("user");
        if (user.ID < 1) throw new ArgumentException("The user ID is invalid.");

        Assignment assignment = null;
        using (var reader = m_assignmentDal.GetByUserID(user.ID))
        {
            if (reader.Read())
            {
                assignment = new Assignment
                {
                    ID = reader.GetInt32(reader.GetOrdinal("AssignmentID")),
                    // ...
                };

                return assignment;
            }
        }
    }
}

class UserDal: IUserDal
{
    public UserDal(DbConnection connection)
    {
        m_connection = connection;
    }

    DbConnection m_connection;

    public User GetByKey(int id)
    {
        using (DbCommand command = connection.CreateCommand())
        {
            command.CommandText = "SELECT * FROM Users WHERE UserID = @UserID";
            var param = command.Parameters.Add("@UserID", DbType.Int32);
            param.Value = id;

            var reader = command.ExecuteReader(CommandBehavior.SingleResult|CommandBehavior.SingleRow|CommandBehavior.CloseConnection);
            return reader;                
        }
    }

    // ...
}

class AssignmentDal: IAssignmentDal
{

    public AssignmentDal(DbConnection connection)
    {
        m_connection = connection;
    }

    DbConnection m_connection;

    Assignment GetByUserID(int userID)
    {
        using (DbCommand command = connection.CreateCommand())
        {
            command.CommandText = "SELECT a.* FROM Assignments a JOIN Users u ON a.AssignmentID = u.AssignmentID WHERE u.UserID = @UserID";
            var param = command.Parameters.Add("@UserID", DbType.Int32);
            param.Value = id;

            var reader = command.ExecuteReader(CommandBehavior.SingleResult|CommandBehavior.SingleRow|CommandBehavior.CloseConnection);
            return reader;                
        }
    }

    // ...
}

// Implement other CRUD services similarly

The conceptual layers and flow of data/objects that result from this architecture would be as follows:

Task:                         UserManagementSvc
                                      ^
                                      |
             -------------------------------------------------
             |              |                 |              |
Entity:  UserSvc   UserAssociationsSvc   AssignmentSvc   LocationSvc
             ^       ^                        ^              ^
             |       |                        |              |
             ---------                        -              -
                 |                            |              |
Utility:      UserDal                   AssignmentDal   LocationDal
                 ^                            ^              ^
                 |                            |              |
                 ---------------------------------------------
                                       |
DB:                             (SQL Database)

A couple key things to note here regarding composition and dependencies. By adding the UserManagementService and composing the entity services within it, you achieve the following:

  1. Elimination of coupling between entity services.
  2. Reduction of the volume of dependencies for each entity service.
    • They are only dependent upon their DAL and possibly common infrastructure.
  3. Dependencies are now uni-directional: All dependencies are 'downward', never 'horizontal' or 'upwards'.
    • This simple rule provides a very clean mechanism by which unruly dependencies can be completely eliminated.
  4. The rules of associating a user with an assignment and a location is removed from the entities and pushed higher.
    • This provides more flexible compositions and encourages code reuse.
    • Other services like UserManagementService may be written that compose User, Assignment, and Location and/or other entities differently to meet different business rules and solve different problems.
  5. Even higher-level services may be written above UserManagementService and similar services, composing them in a similar way, creating even higher-level workflows with minimal effort.

If you are careful in how you design and write each level of services, you can provide many layers of differing responsibility, complexity, and composability. An application becomes less about writing business rules, and more about composing parts to create business behavior. If a part needs to be written, it can usually be written by composing other parts, and possibly adding a small amount of additional behavior. Construction of an application becomes a lot simpler, and it is much easier to build fully functional, self-contained, reusable parts that are easier to test in isolation, and easier to deploy.

You will also have achieved Service-Orientation in the truest sense (according to Thomas Erl anyway), along with all of its benefits. ;)

jrista
Thanks this is starting to make sense. I had one more question. Would i also create a UserManagementDal Object as well that does similar stuff with the Dal Objects?
zSysop
You should explicitly avoid making a UserManagementDal. Note the difference in the names of the different service types: Entity vs. Task vs. Orchestration. An entity service is responsible for the CRUD operations of an entity, and as such, it is responsible for communicating with a data store. However, task, orchestration, application, and other services should NEVER talk to a database...only other services. It all boils down to responsibilities, and separating those responsibilities out into different services. Entities are responsible for low level data store interaction.
jrista
UserManagementService is what Thomas Erl calls a task service. It is responsible for the higher-level "task" of creating a user. Note the difference in the kind of behavior of a task service vs. an entity service. The entity service provides low level CRUD, while the task provides higher-level, milti-entity composite behavior. Orchestration services (or business workflow services) are even higher level, composing multiple task and entity services in even broader chunks of behavior. Application services would compose orch, task, and maybe entity services to perform application specific stuff.
jrista
Entity services should generally be the only thing that communicates with a core entity storage database. Higher level services may utilize their own databases for data specific to those services, but that is rarer. It is of key importance to identify service roles, and encapsulate those different roles appropriately, and at the right level. Keeping dependencies going downward from higher level service compositions to lower level finer grained services is the critical success factor for providing an easy to maintain, flexible, highly reusable library of services.
jrista
zSysop
Each entity service should have its own DAL. The UserService should have one dependency on IUserDal. IUserDal should have methods to do the basic CRUD operations of a User ONYL, nothing else. Since User also has associations to other entities, it DAL should also have methods to manage those entities. I have updated the example to demonstrate. Reread the examples from before to see how the User object moves up through the service layers.
jrista
ahhh makes sense. So it's okay for the reader object to be passed back to the ManagementService. I was always under the assumption that reader objects shouldn't be returned, but if that is okay then this makes alot of sense!Thanks for your all help and the time you've given. It was extremely benefecial! I learned alot.
zSysop
Note, I never passed it back to the management service, only the entity service. I am not sure how else to say it other than: ONLY, ABSOLUTELY ONLY, the ENTITY service can do ANYTHING with the DB. The management service is supposed to be PURELY object oriented, retrieving objects that need to be loaded from the database ONLY through Entity services. Its what we call the Information Expert pattern. Keep the number of communicators with your DB as low as humanly possible, and restrict each entity to a single service. Higher-level services should not, if at all possible, access the DB.
jrista
Reread the whole answer and thoroughly read the examples again. It should be pretty clear what the role of each different kind of service is. It should also be very clear where the buck stops with DB access, and where object-only behavior begins.
jrista
@zSysop: Another thought occurred to me, which might help you understand the rules around DB access better. IUserManagementService is a higher-level composite service. It "composes" behavior from several lower-level entity services. A key factor of SOA is that services should be as autonomous as possible, which makes them distributable. If you need scalability, and IUserManagementService is implemented properly...you can host it in a separate server from the entity services. If you tried to pass an IDataReader out of the entity service, that would fail in a distributed scenario.
jrista
Thanks for being so patient. Things are starting to click.The only thing that is still a little bit unclear to me is the use of the AssociationService. What was your reasoning for including this as a service?
zSysop
Encapsulation of responsibility. Look for the patterns: IUserService, IAssignmentService, ILocationService. They are solely responsible for a SINGLE entity. IUserAssociationService is different. It is responsible for an association between a parent entities and several other entities. You could have included those methods in the IUserService, but then you are suddenly expanding its responsibilities, which in turn expands its dependencies. It is all about reduction and isolation of responsibility in an effort to reduce complexity, improve dependencies, and meet the tenets of SOA.
jrista
Hi jrista,I had one more question for you. Hope you don't mind.Where would validation occur within the design you outlined.Would it be okay for it to reside in the UserManagementService?Thanks once again.
zSysop
It would depend on the kind of validation. You have data integrity validation, and you have business rule validation. Data integrity validation belongs in the entity services. Business rule validation belongs in a business service, such as UserManagementService.
jrista