c# - 当 SRP 违规似乎将复杂性向下传递时的正确方法

Question

在意识到我的类(class)到处都是之后，目前正在进行大量的重构工作。我试图将事情分开一点，以更好地遵循 SRP，但我总是发现很难评估一个类是否有“一个改变的理由”的格言。我希望这个实际例子可以帮助我理解。

有问题的代码旨在清理数据。目前这里有两个独立的进程——我们使用通过代码调用的外部应用程序来清理地址数据。我们使用 C# 中的内部算法清理其他数据字段。

当我被告知我们可能希望在 future 更改这两个进程时，这个重构就开始了——例如，使用数据库存储过程来完成这两项工作，而不是 C# 代码和外部应用程序。所以我的第一 react 是将这两个函数隐藏在接口(interface)后面(FileRow 和 FileContents 只是 DTO):

public interface IAddressCleaner
{
    string CleanAddress(StringBuilder inputAddress);
    void CleanFile(FileContents fc);
}

public interface IFieldCleaner
{
    string CleanPhoneNumber(string phoneToClean);
    void CleanAllPhoneFields(FileRow row, FileContents fc);
    void MatchObscentities(FileRow row, FileContents fc);
    void CleanEmailFields(FileRow row, FileContents fc);
}

这很好。然而，实际上，我无法想象一个类(class)会在没有另一个的情况下使用其中的一个。因此，将它们(及其实现)合并为一个类似乎是有意义的。考虑到我们可能会用单个解决方案(例如数据库)替换这两个功能，这也是有道理的。

另一方面，似乎 IFieldCleaner已经违反了 SRP，因为它正在做三件事:清理电话号码、电子邮件和查找粗鲁的话，所有这些在逻辑上都是不同的过程。因此似乎有理由将其拆分为 IPhoneCleaner , IObscenityMatcher和 IEmailCleaner .

后一种方法让我特别困扰的是，这些类用于服务中，该服务已经具有大量愚蠢的接口(interface)依赖项:

public class ReadFileService : IExecutableObject
{
    private ILogger _log;
    private IRepository _rep;
    private IFileHelper _fileHelp;
    private IFieldCleaner _fieldCleaner;
    private IFileParser _fileParser;
    private IFileWriter _fileWriter;
    private IEmailService _mailService;
    private IAddressCleaner _addressCleaner;

    public ReadFileService(ILogger log, IRepository rep, IFileHelper fileHelp, IFileParser fileParse, IFileWriter fileWrite, IEmailService email, IAddressCleaner addressCleaner)
    {
        // assign to privates
    }

  // functions
}

反过来，这看起来也违反了 SRP 到可笑的程度，而没有向其添加额外的两个接口(interface)。

这里的正确方法是什么？我应该有一个ICleaner接口(interface)，还是一分为五？

Answer 1

免责声明:我不是专家，人们可能不同意我在这里的一些想法。很难提供直接的答案，因为这在很大程度上取决于幕后的情况。可能还有很多“正确”的答案，但这一切都取决于我们在这里缺少的太多信息。尽管如此，还没有人回答，我认为我可以指出一些可能会引导您朝着正确方向前进的事情。

祝你好运!

您可以访问 Pluralsight 吗？买一个快一个月完全值得，只是为了通过Encapsulation and SOLID .我在浏览它时的“啊哈”时刻之一是查看您的方法签名以帮助识别您可以提取的接口(interface)以帮助简化代码。忽略名称，只看参数。

我将尝试使用您提供的代码完成练习，但我需要在此过程中做出可能不正确的假设。

上 IFieldCleaner您有 3 个具有相同签名的方法:

void CleanAllPhoneFields(FileRow row, FileContents fc);
void MatchObscentities(FileRow row, FileContents fc);
void CleanEmailFields(FileRow row, FileContents fc);

请注意这些方法是如何完全相同的。这表明您可以提取具有 3 个实现的单个接口(interface):

interface IFieldCleaner {
  void Clean(FileRow row, FileContents fc);
}
class PhoneFieldCleaner : IFieldCleaner { }
class ObscentitiesFieldCleaner : IFieldCleaner { }
class EmailFieldCleaner : IFieldCleaner { }

现在，这很好地将清理这些字段的职责划分为一口大小的类。

现在您有其他几种清洁方法:

string CleanPhoneNumber(string phoneNumber);
string CleanAddress(StringBuilder inputAddress);

这些非常相似，只是需要一个 StringBuilder大概是因为实现关心各个行？让我们把它换成 string并假设实现将处理行拆分/解析，然后我们得到与之前相同的结果——两个具有相同签名的方法:

string CleanPhoneNumber(string phoneNumber);
string CleanAddress(string inputAddress);

所以，按照我们之前的逻辑，让我们也创建一个与清理字符串相关的接口(interface):

interface IStringCleaner {
  string Clean(string s);
}

class PhoneNumberStringCleaner : IStringCleaner { }
class AddressStringCleaner : IStringCleaner { }

现在我们已经将这些职责分离到它们自己的实现中。

此时，我们只剩下一种方法可以解决:

void CleanFile(FileContents fc);

我不确定这个方法有什么作用。为什么它是 IAddressCleaner 的一部分?所以，现在我不讨论它——也许这是一种读取文件、找到地址、然后清理它的方法，在这种情况下，你可以通过调用我们的新 AddressStringCleaner 来完成。 .

所以让我们看看到目前为止我们在哪里。

interface IFieldCleaner {
  void Clean(FileRow row, FileContents fc);
}

class PhoneFieldCleaner : IFieldCleaner { }
class ObscentitiesFieldCleaner : IFieldCleaner { }
class EmailFieldCleaner : IFieldCleaner { }

interface IStringCleaner {
  string Clean(string s);
}

class PhoneNumberStringCleaner : IStringCleaner { }
class AddressStringCleaner : IStringCleaner { }

这些似乎都与我相似，而且有些气味。基于您的原始方法名称，例如 Clean 全部 字段，看来您可能正在使用循环来清除 FileRow 中的某些列?但是为什么还要靠FileContents ?同样，我看不到你的实现，所以我不太确定。也许您打算传递原始文件或数据库输入？

我也看不到你在哪里店铺 清理后的结果——你之前的大部分方法都返回了 void这意味着调用该方法有一些副作用(即它是一个命令)，而一些方法只返回一个干净的字符串(一个查询)。

所以我假设总体意图是 干净的琴弦无论它们来自何处和 也将它们存储回某个地方。如果是这种情况，我们可以进一步简化我们的模型:

interface IStringCleaner {
  string Clean(string s);
}

class PhoneNumberStringCleaner : IStringCleaner { }
class AddressStringCleaner : IStringCleaner { }
class ObscenitiesStringCleaner : IStringCleaner { }
class EmailStringCleaner : IStringCleaner { }

请注意，我们不再需要 IFieldCleaner ，因为这些字符串清理器只处理要清理的输入字符串。

现在回到您的原始上下文——您似乎可以从文件中获取数据，并且这些文件可能有行？这些行包含我们需要清理其值的列。我们还需要坚持我们所做的清理更改。

因此，根据您提供的服务，我看到一些可能对我们有帮助的事情:

IRepository
IFileHelper
IFileWriter
IFileParser

我的假设是，我们打算将清理过的字段保留回 - 我不确定的地方，因为我看到了一个“存储库”，然后还有一个“FileWriter”。

无论如何，我们知道我们最终需要从字段中取出字符串，也许 IFileParser会有所帮助吗？

interface IFileParser {
  FileContents ReadContents(File file);
  FileRow[] ReadRows(FileContents fc);
  FileField ReadField(FileRow row, string column);
}

这可能比它需要的更复杂-- FileField可以负责存储字段值，因此大概您可以将所有这些组合在一起以形成 FileContents坚持回到磁盘。

所以，现在我们已经将我们的最终目标(干净的东西)与输入的来源(文件、数据库等)以及我们如何持久化(返回到文件、数据库等)分开了。

您现在可以根据需要使用您的服务来组成此流程。比如你说目前你调用一个外部程序来清理地址？没问题:

class ExternalAddressStringCleaner : IStringCleaner {
  // depend on whatever you need here

  public string Clean(string s) {
    // call external program
    return cleanString;
  }
}

现在你切换到存储过程了吗？好的，也没有问题:

class DatabaseAddressStringCleaner : IStringCleaner {

  // depend on database
  DatabaseAddressStringCleaner(IRepository repository) {
  }

  string Clean(string s) {
    // call your database sproc
    return cleanString;
  }
}

很难为您的服务推荐想法——但您可以将其拆分为单独的较小服务( FileReaderService 、 FileCleaningService 和 FileStoreService )或简化您采用的依赖项。

现在您只有一个界面 IStringCleaner ，您可以只声明您需要的清洁器，并随时将它们换出/更改。

public FileCleanerService {
  private IStringCleaner _addressCleaner;
  private IStringCleaner _phoneCleaner;
  private IStringCleaner _obscenityCleaner;
  private IStringCleaner _emailCleaner;

  ctor(IFileParser parser, /* deps */) {
    _parser = parser;

    _addressCleaner = new ExternalAddressStringCleaner(/* deps */);
    _phoneCleaner = new PhoneStringCleaner();
    _obscenityCleaner = new ObscenityStringCleaner();
    _emailCleaner = new EmailStringCleaner();
  }

  public void Clean(FileContents fc) {

    foreach(var row in _parser.ReadRows(fc)) {
      var address = _parser.ReadField(row, "Address");
      var phone   = _parser.ReadField(row, "Phone");
      var post    = _parser.ReadField(row, "PostContent");
      var email   = _parser.ReadField(row, "Email");

      // assumes you want to write back to the field?
      // handle this however you want
      address.Value = _addressCleaner.Clean(address.Value);
      phone.Value = _phoneCleaner.Clean(phone.Value);
      post.Value = _obscenityCleaner.Clean(post.Value);
      email.Value = _emailCleaner.Clean(email.Value);
    }

}

我对您的流程和代码做了很多假设，所以这可能比我假设的要复杂得多。在没有所有信息的情况下，很难提供指导——但是您仍然可以通过查看接口(interface)和名称来推理一些基本的事情，我希望我已经证明了这一点。有时你只需要越过表面就能看到矩阵后面的 1 和 0，然后这一切就有意义了;)

为长篇文章道歉，但我完全理解你来自哪里。弄清楚如何重构事物令人生畏，令人困惑，而且似乎没有人能够提供帮助。希望这能让你在重构时有一个开始的地方。这是一项艰巨的任务，但只要坚持一些简单的指导方针和模式，根据您付出的努力，它最终可能会更容易维护。再说一次，我 绝对推荐 PluralSight 类(class)。