可扩展代码以支持不同的HR规则

最近，我受到挑战，要使用以下项目要点编码：

可扩展代码，以支持人事部门的不同年假规则

可维护的代码，可在不影响其他客户端的情况下添加/更改现有规则

不同客户端的可定制和可配置代码

异常处理和日志记录，以保护代码并简化支持

遵循设计和OOP原则

可单元测试的代码

>

我编写代码时要牢记SOLID原理，这是我的代码。我错过了什么？我无法获得高分。我缺少什么设计策略？

public class EmployeeLeave : ILeaveRequest
    {
        IDataBaseService databaseService;

        public EmployeeLeave(IDataBaseService databaseService)
        {
            this.databaseService = databaseService;
        }


        public void ProcessLeaveRequest(DateTime leaveStartDate, int days, string reason, int employeeId)
        {
            try
            {
                var employee = FindEmployee(employeeId);

                if ((DateTime.Now - employee.StartDate).TotalDays <= 90 && !employee.IsMarried)
                    throw new Exception("Invalid leave request.");

                if (days > 20)
                    throw new Exception("Invalid leave request.");

                var leaveRequest = new EmployeeLeaveDetail();

                leaveRequest.EmployeeId = employeeId;
                leaveRequest.LeaveStartDateTime = leaveStartDate;
                leaveRequest.LeaveEndDateTime = leaveStartDate.AddDays(days);

                SaveLeaveRequest(leaveRequest);

            }
            catch (Exception)
            {

                throw;
            }
        }

        public void SaveLeaveRequest(ILeaveRequestDetail leaveRequest)
        {
            try
            {
                databaseService.ExecuteProcedure("spLocal_InsertIntoEmployees", new List<Parameters>
                {
                   new Parameters {ParameterName="@EmployeeId",
                       ParameterValue =leaveRequest.EmployeeId },

                   new Parameters {ParameterName="@StartDate",
                       ParameterValue =leaveRequest.LeaveStartDateTime },

                   new Parameters {ParameterName="@EndDate",
                       ParameterValue =leaveRequest.LeaveEndDateTime }
                });
            }
            catch (Exception)
            {

                throw;
            }
        }

        public Employee FindEmployee(int employeeId)
        {
            Employee employee = default(Employee);

            try
            {
               

                var sqlReader = databaseService.ExecuteProcedure("spLocal_GetEmployeeFromId", new List<Parameters> { new DBLayer.Parameters {
                ParameterName="@EmployeeId",
                ParameterValue=employeeId
            } });


                using (sqlReader)
                {
                    while (sqlReader.Read())
                    {
                        employee = new Employee
                        {
                            EmployeeId = long.Parse(sqlReader["EmployeId"].ToString()),
                            Name = sqlReader["FirstName"].ToString(),
                            LastName = sqlReader["LastName"].ToString(),
                            StartDate = DateTime.Parse(sqlReader["StartDate"].ToString()),
                            Salary = Decimal.Parse(sqlReader["Salary"].ToString()),
                            IsMarried = bool.Parse(sqlReader["Married"].ToString())
                        };

                    }
                }
            }
            catch (Exception)
            {

                throw;
            }
            return employee;

        }
    }

下面是我的单元测试。

public class AnualLeaveTest
    {
        [TestMethod]
        public void IsMarriedAndGreaterThan90Days()
        {
            //TOdo: Need to check if connection ScopeIdentity returned is same
            EmployeeLeave leave = new EmployeeLeave(new SQLDatabaseService());
            leave.ProcessLeaveRequest(DateTime.Now, 6, "", 212);
        }

        [TestMethod]
        public void IsMarriedAndLessThan90Days()
        {
            //To do: Need to check if connection ScopeIdentity returned is same
            EmployeeLeave leave = new EmployeeLeave(new SQLDatabaseService());
            leave.ProcessLeaveRequest(DateTime.Now, 6, "", 454);
        }

        [TestMethod]
        public void NotMarriedAndGreaterThan90Days()
        {
            //TOdo: Need to check if connection ScopeIdentity returned is same
            EmployeeLeave leave = new EmployeeLeave(new SQLDatabaseService());
            leave.ProcessLeaveRequest(DateTime.Now, 6, "", 767);
        }

        [TestMethod]
        [ExpectedException(typeof(Exception),"Invalid leave request.")]
        public void NotMarriedAndLessThan90Days()
        {
            EmployeeLeave leave = new EmployeeLeave(new SQLDatabaseService());

            leave.ProcessLeaveRequest(DateTime.Now, 6, "", 645);

            
        }

        [TestMethod]
        [ExpectedException(typeof(Exception), "Invalid leave request.")]
        public void LeaveRequestGreaterThan20Days()
        {
            EmployeeLeave leave = new EmployeeLeave(new SQLDatabaseService());

            leave.ProcessLeaveRequest(DateTime.Now, 26, "", 645);


        }
    }

您需要阅读以下内容：企业规则引擎。只是;）

#1 楼

我编写代码时要牢记SOLID原理，这是我的代码。我错过了什么？

让我们看看...

SRP-单一责任原则

一个班级只有一个改变的理由。

被违反。名称EmployeeLeave表示这是一个仅存储有关员工请假的数据的类，但其API却表示其他内容-我是一个存储库。那是什么

您的课程有两个更改的原因：

请求规则
保存请求

OCP-打开/关闭原则

软件实体（类，模块，功能等）应打开以进行扩展，但应关闭以进行修改。

已被破坏。尽管通过DI传递了数据库抽象，但是您已经对所有存储过程的细节进行了硬编码。

IDataBaseService的用户不应该知道他正在使用存储过程或其他任何东西。用户只想保存他的数据。

您不能使用其他存储类型来扩展它。如果删除存储过程，则会破坏该实现。

LSP-Liskov替换原理

子类永远不应破坏父类的类型定义。

不相关。

ISP-接口隔离原理

接口隔离原理（ISP）规定没有客户端应该被强制使用不使用的方法。

已被违反。作为创建请假请求并实现ILeaveRequest的人，我需要实现SaveLeaveRequest和FindEmployee。我不需要我只想创建一个新请求。我不想知道如何保存它（至少不是在这里）。

DIP-依赖关系反转原理

A.高级模块应该不依赖于低级模块。两者都应依赖于抽象。
B.抽象不应依赖于细节。细节应取决于抽象。

违反了。 SaveLeaveRequest依赖于低级存储过程，尽管它使用了抽象IDataBaseService。

摘要

可扩展代码支持不同的HR年假规则部门

失败。您需要为每个请假请求实现保存/查找逻辑。这是很多冗余代码。

可维护的代码，用于添加/更改现有规则，而不影响其他客户端

失败。存储过程调用及其详细信息属于存储库。用户不应该知道它是如何实现的，并且当前他需要确切地知道实现细节才能使用它。

不同客户的可定制和可配置代码

部分遇见。您从数据层入手，但向外界透露了有关存储的更多细节。

异常处理和日志记录可以保护代码，并使支持更容易
以下设计和OOP原则

失败。空的catch不是异常处理。错误消息不是很有帮助。它们不能通过给出原因或提示纠正方法来帮助解决问题。

可测试单元的代码

部分满足。您可以注入另一个数据层，但是如果新数据层不支持硬编码存储过程，则EmployeeLeave的实现将中断。

解决方案（示例）

接口是一个好的开始，但是它太大了，它缺少一些重要的属性，这些属性是ProcessLeaveRequest签名的一部分，但是不应该。

最小的接口应该需要一些基本数据和验证此数据的方法。

interface ILeaveRequest
{
    int EmployeeId { get; }
    DateTime LeaveStartDate { get; }
    int DayCount { get; }
    void Validate();
}

您可以通过实际上仅实现Validate方法（通过DI添加任何其他依赖项）来实现它，例如，如果您需要检查是否员工仍然可以请假。

请注意新的异常类型和消息，它们清楚地说明了请求无效的原因。

class VacationRequest : ILeaveRequest
{
    public VacationRequest(IEmployeeRepository employeeRepository, int employeeId, DateTime leaveStartDate, int dayCount) {..}
    public int EmployeeId { get; }
    public DateTime LeaveStartDate { get; }
    public int DayCount { get; }
    public void Validate()
    {
        // check if employee has enough vacation days...
        throw new OutOfVacationException("Employee 123 does not have any more vacation.");

        // check if max employees have vacation...      
        throw new MaxEmployeeOnVactaionExceededException("The maximum number of 3 employees on vacation at the same time reached.");
    }
}

您可以通过添加更多属性和依赖项来创建更复杂的规则的多个请求-或更少。

这只是一个简单的示例，但在更复杂的解决方案中，您可以拥有一个ILeaveRequestRule，您可以通过DI将它作为多个规则的集合通过DI传递给具体请求，以便您也可以扩展它们。在这种情况下，每个规则都会抛出有意义的异常，以解释违规情况。这完全取决于系统的动态性。如果您认为自己可能需要经常更改，那么可能会顺便去做。

class EmergencyVacationRequest : ILeaveRequest
{
    public EmergencyVacationRequest(IEmployeeRepository employeeRepository, int employeeId, DateTime leaveStartDate, int dayCount) {..}
    public int EmployeeId { get; }
    public DateTime LeaveStartDate { get; }
    public int DayCount { get; }
    public void Validate()
    {
        // check if employee has enough vacation days...
        throw new OutOfVacationException("Employee 123 does not have any more vacation.");

        // other rules might not apply here...
    }
}

要创建所有不同的请假请求，您可以编写一个工厂（最后，一个简单的请假请求处理器将验证每个请求并将其保存在请假存储库中。

class LeaveRequestProcessor
{
    public LeaveRequestProcessor(ILeaveRepository leaveRepository) {..}

    public void ProcessLeaveRequest(ILeaveRequest leaveRequst)
    {
        leaveRequst.Validate(); // You might want to catch this somewhere an log it.

        leaveRepository.SaveLeave(
            leaveRequst.EmployeeId, 
            leaveRequst.LeaveStartDate, 
            leaveRequst.DayCount
        );
    }
}

请假存储库当然可以修改各种表，以便请假请求可以访问此数据以验证其规则。

以下是其中的一些新解决方案的优点：

您可以随时创建新的请假请求，而不会破坏您想要的任何逻辑
您只需实现真正需要的新请假请求
您总是知道什么以及为什么它不起作用
您独立于存储类型
您可以通过模拟存储库来测试所有单元

附录-异常与ValidationResul t

对于这样的事情是否应该使用异常或验证结果，人们有各种各样的看法。

在Validate上抛出异常是一种好习惯吗？（）方法还是返回布尔值的更好方法？
验证数据时抛出异常是好事还是坏主意？

事实上，我同时使用了两种模式。您有时会选择个人或项目的个人偏好。

\ $ \ begingroup \ $
这是一个很好的答案，但是我不得不质疑创建诸如OutOfVacationException之类的异常。让我们假装我们忽略了它，如果整个应用程序崩溃是因为员工没有休假时间吗？
\ $ \ endgroup \ $
–RubberDuck
16年12月21日在1:53

\ $ \ begingroup \ $
我有一个关于employeeId与Employee实际实例的使用的问题。我会将雇员作为参数传递给构造函数，因此在请求中，我可以直接查询有关我可能需要的任何信息。仅通过交出ID，我可能不得不再次访问数据库，以使一名员工（很可能）在创建请求之前已被检索。是否有传递ID或阻止传递员工的特定论点？
\ $ \ endgroup \ $
– Germi
16/12/21在7:20

\ $ \ begingroup \ $
@ t3chb0t异常是异常行为。您正在谈论将它们用于预期的控制流。
\ $ \ endgroup \ $
–RubberDuck
16年12月21日在10:00

\ $ \ begingroup \ $
我们同意不同意@ t3chb0t。我看到需要ValidationResult，您却没有。没关系。
\ $ \ endgroup \ $
–RubberDuck
16年12月21日在10:59

\ $ \ begingroup \ $
您提出的解决方案并不理想。为什么请假请求能够自我验证？您在这里违反了开放/封闭原则-您必须更改源以更改适用于每种类型的规则。调用方还必须知道他们正在处理哪种类型，才能知道可能会抛出哪些异常。我想说这也非常接近LSP违规。
\ $ \ endgroup \ $
–RobH
16年12月21日在12:08

#2 楼

public class EmployeeLeave : ILeaveRequest
...
var leaveRequest = new EmployeeLeaveDetail();

这很令人困惑。顾名思义，ILeaveRequest表示实现该接口的类是“离开请求”（我知道类与接口之间没有“是”关系）。但是随后我们看到一个实际的请假请求，形式为EmployeeLeaveDetail。这个EmployeeLeave类仅处理请假请求。那么，为什么该类在不是请求且未实现与请假请求所期望的一致的接口时实现一个名为ILeaveRequest的接口呢？因此，如果EmployeeLeaveDetail是请假请求（根据变量名称），为什么将其称为EmployeeLeaveDetail而不是EmployeeLeaveRequest？我看到您创建了一个名为ILeaveRequestDetail的接口。您应该将该接口重命名为ILeaveRequest，并将当前ILeaveRequest重命名为更准确的名称。

if ((DateTime.Now - employee.StartDate).TotalDays <= 90 && !employee.IsMarried)
  throw new Exception("Invalid leave request.");

if (days > 20)
  throw new Exception("Invalid leave request.");

90和20的意义是什么？？请勿使用幻数，而要创建命名良好的常量，而应使用常量。

也如Heslacher所说，请勿抛出Exception；使用更特定的类型之一或创建自己的特定于域的异常。至少要说出问题所在，否则当您得到Exception时，您会挠头问问（可能）20项检查中的哪一项失败，并且只说Invalid leave request。

public void ProcessLeaveRequest(DateTime leaveStartDate, int days, string reason, int employeeId)

我对这种方法的名称有些疑问，因为它没有处理（无论意味着什么）“离开请求”。您在SaveLeaveRequest(ILeaveRequestDetail)下面有一个方法，它实际上处理请假请求。 ProcessLeaveRequest仅处理整数，字符串等。

我想这里应该发生的事情是，您在某个类中有一个方法可以为员工创建请假请求。该方法接受整数，字符串等，进行验证，然后返回请假请求。然后，您可以调用Save方法保存请求。

通常，您还应该更多地使用域构造。例如，您正在接受int作为员工ID。为什么？当然，这时您应该已经构造了Employee-如果不选择要为其创建请求的Employee，就不可能创建请求-因此，在创建请求时，您只需传递Employee即可，而无需查找它并可能在创建请求时失败。

可扩展代码以支持HR
部门的不同年假规则。
可维护的代码，用于添加/更改现有规则
，而不会影响其他客户端。

您的规则：

public void ProcessLeaveRequest(DateTime leaveStartDate, int days, string reason, int employeeId)
{
  if ((DateTime.Now - employee.StartDate).TotalDays <= 90 && !employee.IsMarried)
      throw new Exception("Invalid leave request.");

  if (days > 20)
      throw new Exception("Invalid leave request.");

以何种方式将规则硬编码到创建和验证假期请求的单一方法中就可以满足那些要求？您完全忽略了需求1.

总的来说，考虑到所有数据库工作，该类应该像一个存储库。

#3 楼

catch (Exception)
{

    throw;
}

好吧，除了多几行代码之外，您什么都买不到。只需完全卸下try..catch。如果您根本没有try..catch，则此构造只是让异常使调用堆栈冒泡。

public void ProcessLeaveRequest

您无需验证所有内容给定方法参数中的一个。
根本不使用reason参数。
您将抛出Exception而不是例如ArgumentOutOfRangeException，这将是更合适的方法。
（从FindEmployee()方法中），Employee是一个结构（提示default(Employee)），这使您有可能向数据库中添加不属于该数据库的内容。假设您将不存在的epmloyeeId传递到FindEmployee()方法中，则会得到一个default(Employee)返回，在结构的情况下，它将具有StartDate和IsMarried的默认属性值。如果此属性在您的验证部分中视为有效，那么对于您不存在的雇员，您将在数据库中得到一个EmployeeLeaveDetail。

您应始终使用大括号{}尽管它们可能是可选的。如果不使用它们，就有可能引入难以跟踪的错误。我想鼓励您始终使用它们。这将帮助您减少代码出错的可能性。
用于if，else if和else。

\ $ \ begingroup \ $
我一直想知道...当我只想将它扔给更高功能时，包装在try / catch中有什么意义？
\ $ \ endgroup \ $
–阿卜杜勒
16/12/20在15:30

\ $ \ begingroup \ $
@Abdul，如果您所做的只是抛出更高的结果，那么包装try / catch毫无意义。仅在要对异常进行处理（添加信息，对其重新分类或对其进行处理）时才捕获异常。
\ $ \ endgroup \ $
–纤巧
16 Dec 20'在17:09

#4 楼

这里有很多有用的评论，但没有人评论DateTime的用法。

EmployeeLeave

DateTime.Now - employee.StartDate

我建议您将所有日期都作为UTC进行处理，并将其仅转换为本地日期需要显示的时间。即使您起诉您的应用程序将在一个时区中使用，但如果您致力于支持时区，那么将来也可以避免很多麻烦。第二个正在测试中，我将在最后再讨论。

该类的另一个问题是它做得太多。您违反了SRP（单一责任原则）。该课程处理假期，寻找员工并验证假期的正确性。我认为您应该将其分为几个类，每个类负责简单的任务。然后，您可以将它们注入到EmployeeLeave类中，并仅通过调用特定方法来构造逻辑。

public EmployeeLeave(IEmployeeLeaveStore employeeLeaveStore, 
                     IEmployeeFinder emploeeFinder, 
                     IHolidayValidator holidayValidator)
{
        if (employeeLeaveStore == null) throw new ArgumentNullException(nameof(employeeLeaveStore));
        if (employeeFinder == null) throw new ArgumentNullException(nameof(employeeFinder));
        if (holidayValidator == null) throw new ArgumentNullException(nameof(holidayValidator));

        this.employeeLeaveStore = employeeLeaveStore;
        this.employeeFinder = employeeFinder;
        this.holidayValidator = holidayValidator;
}

每个接口在这里都是所谓的角色接口，仅具有方法（方法）担任此特定角色。因此，在上面的示例中：

IEmployeeLeaveStory-仅包含保存员工请假对象的方法

IEmployeeFinder-一种方法Find

IHolidayValidator-一种方法IsValid

我将有用的方法是创建单个规则来验证假期的正确性，然后编写一个汇总的假期验证器仅对其子级执行IsValid。看起来可能像这样：

var compositeValidator = new CompositeLeaveValidator(
    new NoMoreThanTwentyDaysValidator(), 
    new MarriedAndEmployedForLessThan3MonthsValidator());

由于可能适用不同的规则，您还可以基于员工类型创建组成。这也是扩展新规则的好方法。

此外，在构造函数中，我们检查所有参数都不为null，如果是Fail fast，这也是一件好事。 />

public void ProcessLeaveRequest(DateTime leaveStartDate, int days, string reason, int employeeId)
{
    var employee = employeeFinder.Find(employeeId);

   if (!holidayValidator.IsValid(leaveStartDate, days, reason, employee))    
       throw new InvalidHolidayException("Specified holiday is invalid.")

    var leaveRequest = new EmployeeLeaveDetail();

    leaveRequest.EmployeeId = employeeId;
    leaveRequest.LeaveStartDateTime = leaveStartDate;
    leaveRequest.LeaveEndDateTime = leaveStartDate.AddDays(days);

    employeeLeaveStore.Save(leaveRequest);
}

我还想将EmployeeLeaveDetail的创建内容提取到一个单独的类中，但这取决于您。

如上所述，DateTime还有一个问题。这次是在单元测试中。

UnitTest

基本上是由于您在DateTime.Now中使用UtcNow（或您应该使用的ProcessLeaveRequest）这一事实，这意味着每次运行测试此方法，您可以将不同的测试作为DateTime运行。更好的方法是按照以下方式创建一个SystemTime。

public static class SystemTime
{
    public static Func<DateTime> Now = () => DateTime.UtcNow;
}

然后在测试的后面，您可以指定测试应该在什么时间进行执行并且在运行测试时不要依赖DateTime。

[TestMethod]
public void IsMarriedAndLessThan90Days()
{
    SystemTime.Now = () => new DateTime(2016,12,20, 0,0,0);
    // do the testing with DateTime fixed on 20th of December 2016.
}

您还可以在需要使用DateTime的任何地方使用此类。这样，您可以确保在所有使用UTC或非UTC的地方都保持一致。

另外检查.NET开发人员的五种常见的夏时制反模式，因为这样做时DST可能存在一些问题在DateTime上进行计算。

#5 楼

一般而言，单元测试应遵循“安排，执行，声明”模式。您的只有“安排，执行”部分。您没有声明任何内容（希望引发异常的除外）。

[TestMethod]
public void IsMarriedAndLessThan90Days()
{
    // Arrange
    EmployeeLeave leave = new EmployeeLeave(new SQLDatabaseService());
    // Act
    leave.ProcessLeaveRequest(DateTime.Now, 6, "", 454);
    // Assert?
}

您的单元测试取决于您的数据库。您应该尝试避免单元测试中的任何外部依赖性。

与以上几点有关：您正在与现有用户一起测试功能。如果数据库中发生任何更改（有人结婚，开始日期已超过90天，...），则必须重写测试。您应该在测试用例中显式构造Employees，以使它们不会发生变化，并且其他人可以轻松查看正在发生的情况。

我认为您的EmployeeLeaveRequest非常了解其运行情况被保存。您的IDatabaseService接口应该具有类似void SaveLeaveRequest(ILeaveRequest request)的方法，您可以在其中移交请求，并使其处理所有SQL或保存请求所需的任何操作。与FindEmployee相似。从数据库中检索员工记录不是EmployeeLeaveRequest的责任。

#6 楼

这是一个很棒的解释。我只是在这里使用策略看到了一个可扩展的观点，即是否可以根据不同的处理类型进行工作，例如永久雇员，承包商等。

所以在这里我们可以将处理器策略用于不同类型的叶子和不同类别，可以由工厂退还。并且ProcessLeaveRequest可以采用策略接口作为输入，也可以调用leaveprocessorfactory以返回正确的策略。

我希望我在这里有一定的道理。

编程黑洞网