有问题的代码旨在清理数据.目前这里有两个独立的进程 – 我们通过使用通过代码调用的外部应用程序来清理地址数据.我们使用C#中的内部算法清理其他数据字段.
当我被告知我们可能希望将来更改这两个进程时,这个重构就开始了 – 例如使用数据库存储过程来执行这两个作业而不是C#代码和外部应用程序.所以我的第一直觉是将这两个函数隐藏在接口后面(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);
}
哪个好.然而,实际上,我无法想象一个班级会在没有其他班级的情况下使用其中一个.因此将它们(及其实现)合并到一个类中似乎是有意义的.考虑到我们可能用一个解决方案(如数据库)替换这两个函数,这也是有意义的.
另一方面,似乎IFieldCleaner已经违反了SRP,因为它正在做三件事：清理电话号码,发送电子邮件和寻找粗鲁的单词,所有这些都是逻辑上不同的过程.因此似乎有理由将其拆分为IPhoneCleaner,IObscenityMatcher和IEmailCleaner.
对后一种方法特别困扰的是这些类在服务中使用,该服务已经具有愚蠢的接口依赖性：
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到一个荒谬的程度,而没有增加额外的两个接口.
这里的方法是什么？我应该有一个ICleaner接口,还是将其分成五个？
解决方法
祝你好运！
你有权使用Pluralsight吗？购买快速的月份是完全值得的,只需要经历Encapsulation and SOLID.我经历过的“啊哈”时刻之一就是看看你的方法签名,以帮助识别你可以提取的界面,以帮助简化代码.忽略名称,只需查看参数即可.
我将尝试使用您提供的代码进行练习,但我需要在可能不正确的方式上做出假设.
在IFieldCleaner上,您有3个具有相同签名的方法：
void CleanAllPhoneFields(FileRow row,FileContents fc); void MatchObscentities(FileRow row,FileContents fc); void CleanEmailFields(FileRow row,FileContents fc);
注意这些方法是如何完全相同的.这表明您可以使用3个实现提取单个接口：
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 CleanPhoneNumber(string phoneNumber); string CleanAddress(string inputAddress);
因此,按照我们之前的逻辑,让我们创建一个与清理字符串相关的接口：
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 { }
这些似乎都与我相似,闻起来有些气味.根据您的原始方法名称(如CleanAllFields),您可能正在使用循环来清除FileRow中的某些列？但为什么还要依赖FileContents？再说一次,我看不到你的实现,所以我不太确定.也许您打算传递原始文件或数据库输入？
我也无法看到存储清理结果的位置 – 大多数先前的方法返回void,这意味着调用方法有一些副作用(即它是一个Command),而一些方法只返回一个干净的字符串(一个Query).
因此,我将假设整体意图是清理字符串,无论它们来自何处,并将它们存储在某处.如果是这种情况,我们可以进一步简化我们的模型：
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);
}
}
我对你的过程和代码做了很多假设,所以这可能比我想象的要复杂得多.没有所有信息,很难提供指导 – 但是通过查看界面和名称仍然可以推断出基本的东西,我希望我已经证明了这一点.有时你只需要看过表面看到矩阵后面的1和0然后它们都有意义;)
为长篇文章道歉,但我完全理解你来自哪里.弄清楚如何重构事物是令人生畏,令人困惑的,似乎没有人能够提供帮助.希望这能让你在重构时有所作为.这是一项艰巨的任务,但只是坚持一些简单的指导方针和模式,并且根据你投入的努力,最终可能会更容易维护.再次,我绝对推荐PluralSight课程.
