Entering the classroom
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 the DAL classes that look like
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 bad design was thought to be talking about a service layer with other Service Layer objects and Dal talking to other Dal objects?
Thank you in advance
source to share
Given the design of your examples, you will run into what I like to call dependent hell. This is definitely an opportunity to go down the route you are taking, but it will result in a highly advanced architecture that will probably be very difficult to maintain and reorganize. However, if you abstract a little, you can simplify your architecture, organize responsibilities a little more, and separate concerns in a way that makes managing your dependencies much easier.
UserService, AssignmentService and LocationService are similar to CRUD style services. A more appropriate term for these would be Entity Services. The entity service should be solely responsible for the closest object's CRUD operations, nothing else. Operations that are related to multiple entities, relationships between entities, etc. can be carried over to a higher level service that can organize larger scale operations. They 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 area of responsibility and control dependencies. Simplify your service contracts to reduce the responsibilities of 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 destination will be owned by the UserManagementService, which will constitute 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 data / object flow that arise from this architecture will be as follows:
Task: UserManagementSvc
^
|
-------------------------------------------------
| | | |
Entity: UserSvc UserAssociationsSvc AssignmentSvc LocationSvc
^ ^ ^ ^
| | | |
--------- - -
| | |
Utility: UserDal AssignmentDal LocationDal
^ ^ ^
| | |
---------------------------------------------
|
DB: (SQL Database)
Several key points to note here regarding composition and dependencies. By adding UserManagementService and composing entity services in it, you achieve the following:
- Excluding communication between entity services.
- Reduced dependencies for each entity service.
- They only depend on their DAL and possibly the general infrastructure.
- Dependencies are now unidirectional: all dependencies are "downward", never "horizontal" or "up".
- This simple rule provides a very clean mechanism whereby messy dependencies can be completely eliminated.
- The rules for associating a user with a destination and location are removed from the entities and pushed above.
- This allows for more flexible compositions and encourages code reuse.
- Other services, such as UserManagementService, can be written that make up User, Assignment, and Location and / or other objects in different ways to fit different business rules and solve different problems.
- Even higher-level services can be written above UserManagementService and similar services, composing them in a similar way, creating even higher workflows with minimal effort.
If you are careful in how you design and write each level of service, you can provide many levels of varying responsibilities, complexity, and merging ability. The app is becoming less about writing business rules and creating parts to create business behavior. If a part needs to be written, it can usually be written down by composing other parts and possibly adding a small amount of additional behavior. Building an application becomes much easier, and it is much easier to create fully functional, self-contained, reusable pieces that are easier to test in isolation and easier to deploy.
You have also achieved Service-Orientation in the literal sense (according to Thomas Erl anyway) and all its benefits.;)
source to share
I don't know what you mean by Dal, but in general you need to balance the cohesion and coupling levels in your application.
In this case, I am wondering why the UserService instance can validate other IUsers. It looks like a connection that can be problematic. However, I do not know your statement, so I could be wrong.
Cross out the class diagram so you can easily see the relationship between the classes.
source to share
You can follow a model similar to the DataContext model followed by LINQ2SQL or Entityframework i.e. have a context that keeps track of objects like "User", etc. In turn, your service layer simply talks to the context to either request or perform operations on all objects if needed. Thus, your individual entities, such as the "User" object, should not have direct knowledge (links) with other "unrelated" objects, and the Context provides the entities to the client.
source to share