I just came across some code where I needed to fix a couple of issues in. Since I am a strong proponent of continuous improvement and refactoring I thought I blog about this good example.
Figure: Webform that allows management of employee type history: "Assign" (Add), "Edit" (Modify) and "Delete"
The purpose of this class "EmployeePositionAssignment" is: helper functions for a webpage that lets you manage the depot history of an employee
Read this and find the code smell!
Keep in mind:
- Single Responsibility Principle
- Functions should be small and express intent
- Purpose of commented code
- Parameter of functions
using System; using System.Linq; using NorthWind.FOMS.Domain; using NorthWind.FOMS.Domain.Rules; namespace NorthWind.FOMS.WebUI.DynamicData.CustomPages.EmployeePositionHistoryAssignments { public class EmployeePositionAssignment { private NorthWindDataContext db = new NorthWindDataContext(); private static readonly TimeSpan defaultTimeSpan = new TimeSpan(1, 0, 0, 0); private static readonly DateTime defaultMaxDate = new DateTime(2100, 1, 1); public EmployeePositionAssignment() { db.Connection.Open(); } //~EmployeePositionAssignment() //{ // if (db.Connection.State == ConnectionState.Open) // { // db.Connection.Close(); // } // db.Dispose(); //} public void UpdateEmployeePositionHistory(Guid EmployeeAssignmentID, Guid EmployeeTypeID, DateTime startDate) { var _positionAssignment = GetCurrentAssignment(EmployeeAssignmentID); var assignmentBefore = this.GetAssignStartDateBeforeEmployeePositionAssigment(_positionAssignment.EmployeeID, _positionAssignment.DateBegin); var assignmentAfter = this.GetAssignStartDateAfterEmployeePositionAssigment(_positionAssignment.EmployeeID, _positionAssignment.DateBegin); if (_positionAssignment != null) { _positionAssignment.EmployeeTypeID = EmployeeTypeID; _positionAssignment.DateBegin = startDate; if (assignmentBefore != null) { assignmentBefore.DateEnd = startDate.AddDays(-1); } if (assignmentAfter == null) { _positionAssignment.DateEnd = defaultMaxDate; } else { _positionAssignment.DateEnd = assignmentAfter.DateBegin.AddDays(-1); } } db.SubmitChanges(); this.ApplyPositionChangesToEmployee(_positionAssignment.EmployeeID); RuleManager.MarkEmployeeShiftAsDirty(_positionAssignment.EmployeeID, _positionAssignment.DateBegin, (DateTime)_positionAssignment.DateEnd); } public void DeleteEmployeePositionHistory(Guid EmployeeAssignmentID) { var employeePositionAssignment = GetCurrentAssignment(EmployeeAssignmentID); if (employeePositionAssignment != null) { Guid EmployeeID = employeePositionAssignment.EmployeeID; var assignmentAfter = this.GetAssignStartDateAfterEmployeePositionAssigment(employeePositionAssignment.EmployeeID, employeePositionAssignment.DateBegin); var assignmentBefore = this.GetAssignStartDateBeforeEmployeePositionAssigment(employeePositionAssignment.EmployeeID, employeePositionAssignment.DateBegin); if (assignmentBefore != null) { assignmentBefore.DateEnd = employeePositionAssignment.DateEnd; } if (assignmentBefore != null && assignmentAfter != null) { if (assignmentAfter.EmployeeTypeID == assignmentBefore.EmployeeTypeID) { assignmentBefore.DateEnd = assignmentAfter.DateEnd; db.EmployeePositionHistoryAssignments.DeleteOnSubmit(assignmentAfter); } } db.EmployeePositionHistoryAssignments.DeleteOnSubmit(employeePositionAssignment); db.SubmitChanges(); this.ApplyPositionChangesToEmployee(EmployeeID); RuleManager.MarkEmployeeShiftAsDirty(EmployeeID, employeePositionAssignment.DateBegin, (employeePositionAssignment.DateEnd.HasValue ? employeePositionAssignment.DateEnd.Value : defaultMaxDate)); } } public void AssignEmployeePositionHistory(Guid EmployeeID, Guid EmployeeTypeID, DateTime startDate) { if (!IsExistedEmployeePositionAssignment(EmployeeID, startDate)) { var assignmentAfter = GetAssignStartDateAfterEmployeePositionAssigment(EmployeeID, startDate); var assignmentBefore = GetAssignStartDateBeforeEmployeePositionAssigment(EmployeeID, startDate); EmployeePositionHistoryAssignment epha = new EmployeePositionHistoryAssignment(); epha.EmployeePositionHistoryAssignmentID = Guid.NewGuid(); epha.EmployeeTypeID = EmployeeTypeID; epha.EmployeeID = EmployeeID; if (assignmentAfter != null && assignmentBefore != null) { assignmentBefore.DateEnd = startDate.Subtract(defaultTimeSpan); epha.DateBegin = assignmentBefore.DateEnd.Value.Add(defaultTimeSpan); epha.DateEnd = assignmentAfter.DateBegin.Subtract(defaultTimeSpan); } else if (assignmentAfter != null && assignmentBefore == null) { epha.DateEnd = assignmentAfter.DateBegin.Subtract(defaultTimeSpan); epha.DateBegin = startDate; } else if (assignmentBefore != null & assignmentAfter == null) { epha.DateBegin = startDate; assignmentBefore.DateEnd = (DateTime?)startDate.Subtract(defaultTimeSpan); epha.DateEnd = defaultMaxDate; } db.EmployeePositionHistoryAssignments.InsertOnSubmit(epha); db.SubmitChanges(); ApplyPositionChangesToEmployee(EmployeeID); RuleManager.MarkEmployeeShiftAsDirty(EmployeeID, (DateTime)epha.DateBegin, (DateTime)epha.DateEnd); } } public void ApplyPositionChangesToEmployee(Guid EmployeeID) { var positionChange = (from positionAssignment in db.EmployeePositionHistoryAssignments where positionAssignment.EmployeeID == EmployeeID && positionAssignment.DateBegin <= DateTime.Today && positionAssignment.DateEnd >= DateTime.Today select positionAssignment).FirstOrDefault(); if (positionChange != null) { positionChange.Employee.EmployeeTypeID = positionChange.EmployeeTypeID; db.SubmitChanges(); } } public bool IsExistedEmployeePositionAssignment(Guid EmployeeID, DateTime startDate) { var positionAssignment = (from assignment in db.EmployeePositionHistoryAssignments where assignment.DateBegin == startDate && assignment.EmployeeID == EmployeeID select assignment).FirstOrDefault(); return positionAssignment == null ? false : true; } public bool ValidateStartDate(Guid EmployeeID, DateTime startDate) { //Dev1: Fixed. don't allow crossover date var assignmentAfter = GetAssignStartDateAfterEmployeePositionAssigment(EmployeeID, startDate); var assignmentBefore = GetAssignStartDateBeforeEmployeePositionAssigment(EmployeeID, startDate); if (assignmentAfter != null) { if (assignmentAfter.DateBegin <= startDate) { return false; } } if (assignmentBefore != null) { if (assignmentBefore.DateBegin >= startDate) { return false; } } return true; } public bool ValidateEmployeeType(Guid EmployeeID, Guid EmployeeTypeID, DateTime startDate) { var assignmentAfter = GetAssignStartDateAfterEmployeePositionAssigment(EmployeeID, startDate); var assignmentBefore = GetAssignStartDateBeforeEmployeePositionAssigment(EmployeeID, startDate); if ((assignmentAfter != null && assignmentAfter.EmployeeTypeID == EmployeeTypeID) || (assignmentBefore != null && assignmentBefore.EmployeeTypeID == EmployeeTypeID)) { return false; } else { return true; } } public bool ValidationOnDeletedAssignment(Guid EmployeeID) { //Dev2: Fixed. Don't allow to delete the last record. int count = (from assignment in db.EmployeePositionHistoryAssignments where assignment.EmployeeID == EmployeeID select assignment).Count(); if (count > 1) { return true; } else { return false; } } public bool VidationDateBeginOnUpdatedAssingment(Guid employeeAssingmentID, DateTime startDate) { //Dev1: the begin date of the first record should always be in the past var positionAssignment = (from assignment in db.EmployeePositionHistoryAssignments where assignment.EmployeePositionHistoryAssignmentID == employeeAssingmentID select assignment).FirstOrDefault(); if (positionAssignment != null) { var firstAssignment = this.GetFirstPositionHistoryAssignment(positionAssignment.EmployeeID); if (firstAssignment.EmployeePositionHistoryAssignmentID == positionAssignment.EmployeePositionHistoryAssignmentID) { if (startDate > DateTime.Today) { return false; } } return true; } else { return false; } } // --- SNIP SNIP SNIP more of code godness private static int ApplyDepotChangesToShifts(NorthWindDataContext dataContext, EmployeeDepotHistoryAssignment assignment, bool isDelete, DateTime originalDateBegin) { var existingSameBeforeOne = dataContext.EmployeeDepotHistoryAssignments.Where(h => h.DateBegin < assignment.DateBegin && h.EmployeeID == assignment.EmployeeID).OrderByDescending(h => h.DateBegin).FirstOrDefault(); Guid depotid = assignment.DepotID; DateTime dateBegin = assignment.DateBegin; DateTime dateEnd = assignment.DateEnd.HasValue ? assignment.DateEnd.Value.AddDays(1) : DateTime.MaxValue; Guid employeeID = assignment.EmployeeID; Guid previouseDepotID = existingSameBeforeOne == null ? Guid.Empty : existingSameBeforeOne.DepotID; DateTime previouseDateEnd = existingSameBeforeOne == null ? originalDateBegin : (existingSameBeforeOne.DateEnd.HasValue ? existingSameBeforeOne.DateEnd.Value.AddDays(1) : DateTime.MaxValue); int affectedShifts = 0; //dataContext.MoveShifts(assignment.DepotID, previouseDepotID, dateBegin, dateEnd, employeeID); if (isDelete) { affectedShifts = MoveShifts(dataContext, assignment.DepotID, previouseDepotID, dateBegin, dateEnd, employeeID); if (existingSameBeforeOne != null) { existingSameBeforeOne.DateEnd = assignment.DateEnd; } dataContext.EmployeeDepotHistoryAssignments.DeleteOnSubmit(assignment); dataContext.SubmitChanges(); RuleManager.MarkEmployeeShiftAsDirty(employeeID, dateBegin, dateEnd); } else { if (existingSameBeforeOne != null) { existingSameBeforeOne.DateEnd = assignment.DateBegin.AddDays(-1); } dataContext.SubmitChanges(); if (assignment.DateBegin <= previouseDateEnd) { //if (assignment.DateBegin < DateTime.Today.AddDays(1)) //{ affectedShifts = MoveShifts(dataContext, previouseDepotID, assignment.DepotID, assignment.DateBegin, previouseDateEnd, employeeID); //} //if (assignment.DateBegin < DateTime.Today.AddDays(1)) //{ RuleManager.MarkEmployeeShiftAsDirty(employeeID, assignment.DateBegin, previouseDateEnd); //} } else { //if (assignment.DateBegin < DateTime.Today.AddDays(1)) //{ affectedShifts = MoveShifts(dataContext, assignment.DepotID, previouseDepotID, previouseDateEnd, assignment.DateBegin, employeeID); //} //if (assignment.DateBegin < DateTime.Today.AddDays(1)) //{ RuleManager.MarkEmployeeShiftAsDirty(employeeID, previouseDateEnd.AddDays(-1), assignment.DateBegin.AddDays(1)); //} } } //Update employee's deopt var currentAssignment = dataContext.EmployeeDepotHistoryAssignments.Where(h => h.DateBegin <= DateTime.Today && (h.DateEnd.HasValue ? h.DateEnd.Value : DateTime.MaxValue) > DateTime.Today && h.EmployeeID == employeeID).FirstOrDefault(); if (currentAssignment != null) { Employee emp = dataContext.Employees.Where(e => e.EmployeeID == employeeID).FirstOrDefault(); if (emp != null) { emp.Depot = dataContext.Depots.Single(d => d.DepotID == currentAssignment.DepotID); //emp.DepotID = currentAssignment.DepotID; dataContext.SubmitChanges(); } } return affectedShifts; } // SNIP SNIP SNIP -> I spare you the rest } }
I post the refactored code file in the next days. You can and should read the book: "Clean Code" from Uncle Bob in the meantime
No comments:
Post a Comment