在这段代码中找不到只是重构代码的错误

Question

我有一段 C# 代码，我打算对其进行重构并使其变得更好 - 遵守 SOLID、DRY、KISS 等标准。原代码是：

public class CustomerService
    {
        public bool AddCustomer(string firname, string surname, string email, DateTime dateOfBirth, int companyId)
        {
            if (string.IsNullOrEmpty(firname) || string.IsNullOrEmpty(surname))
            {
                return false;
            }

            if (!email.Contains("@") && !email.Contains("."))
            {
                return false;
            }

            var now = DateTime.Now;
            int age = now.Year - dateOfBirth.Year;
            if (now.Month < dateOfBirth.Month || (now.Month == dateOfBirth.Month && now.Day < dateOfBirth.Day)) age--;

            if (age < 21)
            {
                return false;
            }

            var companyRepository = new CompanyRepository();
            var company = companyRepository.GetById(companyId);

            var customer = new Customer
                               {
                                   Company = company,
                                   DateOfBirth = dateOfBirth,
                                   EmailAddress = email,
                                   Firstname = firname,
                                   Surname = surname
                               };

            if (company.Name == "VeryImportantClient")
            {
                // Skip credit check
                customer.HasCreditLimit = false;
            }
            else if (company.Name == "ImportantClient")
            {
                // Do credit check and double credit limit
                customer.HasCreditLimit = true;
                using (var customerCreditService = new CustomerCreditServiceClient())
                {
                    var creditLimit = customerCreditService.GetCreditLimit(customer.Firstname, customer.Surname, customer.DateOfBirth).Result;
                    creditLimit = creditLimit*2;
                    customer.CreditLimit = creditLimit;
                }
            }
            else
            {
                // Do credit check
                customer.HasCreditLimit = true;
                using (var customerCreditService = new CustomerCreditServiceClient())
                {
                    var creditLimit = customerCreditService.GetCreditLimit(customer.Firstname, customer.Surname, customer.DateOfBirth).Result;
                    customer.CreditLimit = creditLimit;
                }
            }

            if (customer.HasCreditLimit && customer.CreditLimit < 500)
            {
                return false;
            }

            CustomerDataAccess.AddCustomer(customer);

            return true;
        }
    }
}

我将其重构如下：

    public bool AddCustomer(string firname, string surname, string email, DateTime dateOfBirth, int companyId)
    {
        if (!_customerValidator.ValidateCustomer(firname, surname, email, dateOfBirth))
        {
            return false;
        }

        var company = _companyRepository.GetById(companyId);
        var customer = _customerFactory.CreateCustomer(firname, surname, email, dateOfBirth, company);

        customer.HasCreditLimit = _creditLimitCalculator.AssessCreditLimit(company.Name);
        customer.CreditLimit = _creditLimitCalculator.RetrieveCreditLimit(customer);

        if (!_creditLimitValidator.HasCreditLimit(customer))
        {
            return false;
        }

        _customerDataAccessFactory.AddCustomer(customer);

        return true;
    }
}

我基本上希望类只调用方法来尽可能满足 SOLID 和 DRY 的要求。

当我提交这段代码时，它是根据一系列未提供给我们的测试来判断的，我被告知的错误是：

There was a mistake that made the credit check work in reverse, which broke business logic. 

处理它的新代码的相关部分只是从原始代码中删除，基本上在这里：

public class CreditLimitCalculator : ICreditLimitCalculator
{
    public bool AssessCreditLimit(string companyName)
    {
        if (companyName == Company.VeryImportantClient)
        {
            return true;
        }
        return false;
    }

    public int RetrieveCreditLimit(Customer customer)
    {
        int creditLimit;
        switch (customer.Company.Name)
        {
            case Company.VeryImportantClient:
                creditLimit = customer.CreditLimit;
                break;

            case Company.ImportantClient:
                using (var customerCreditService = new CustomerCreditServiceClient())
                {
                    var limit = customerCreditService.GetCreditLimit(customer.Firstname, customer.Surname, customer.DateOfBirth).Result;
                    limit *= 2;
                    creditLimit = limit;
                }

                break;

            default:
                using (var customerCreditService = new CustomerCreditServiceClient())
                {
                    var limit = customerCreditService.GetCreditLimit(customer.Firstname, customer.Surname, customer.DateOfBirth).Result;
                    creditLimit = limit;
                }
                break;
        }

        return creditLimit;
    }
}

并且：

public bool HasCreditLimit(Customer customer)
{
    if (customer.HasCreditLimit && customer.CreditLimit < 500)
    {
        return false;
    }

    return true;
}

它基本上是完全相同的代码，只是转移到不同的类中，所以我不知道为什么它会失败。它也无法初始化，因此也无法调试和单步执行它。有什么建议吗？

Answer 1

我认为访问客户的 CreditLimit 会触发测试工具中的行为，这被认为是不正确的，因为原始代码在公司非常重要的情况下不会触发

在您的代码中，无论公司的评估如何，您都设置信用限额：

customer.CreditLimit = _creditLimitCalculator.RetrieveCreditLimit(customer);

您的 RetrieveCreditLimit(customer) 方法返回 customer.CreditLimit 对于非常重要的公司：

case Company.VeryImportantClient:
            creditLimit = customer.CreditLimit;
            break;

对于这种情况，简化为

customer.CreditLimit = customer.CreditLimit;

现在，我同意看起来是良性的，但我们无法判断 customer.CreditLimit 被初始化为什么，或者 set/get 访问器是否执行比仅访问底层属性更复杂的操作。例如，如果他们在某处触发了某些操作 - 例如监视设置/访问客户信用限额的尝试，然后它将围绕此代码向系统公开一组不同的调用，因为：

在原始代码中，在公司非常重要的情况下，不会尝试设置或获取客户。CreditLimit：

if (company.Name == "VeryImportantClient")
        {
            // Skip credit check
            customer.HasCreditLimit = false;
        }

即使在原始代码的末尾，也不会计算下面的第二个子句：

if (customer.HasCreditLimit && customer.CreditLimit < 500)
        {
            return false;
        }

所以我最初的猜测是，访问客户的 CreditLimit 会触发测试工具中的行为，这被认为是不正确的，因为原始代码在公司非常重要的情况下不会触发

您有权访问 Customer 类源代码吗？它对 CreditLimit 的访问有什么特别的吗？

如果客户有CreditLimit，您可以尝试仅设置CreditLimit吗？

if (customer.HasCreditLimit)
{
    customer.CreditLimit = _creditLimitCalculator.RetrieveCreditLimit(customer);
}