重构:勿以善小而不为

简介: 重构:勿以善小而不为

重构最大的敌人不是技巧与能力,而是懒惰,或者说是态度。
许多细小的重构看似无足轻重,例如方法重命名,提取方法,即使重构了,似乎对代码的结构也没有太大的影响,于是就决定淡然处之,心里想“事情还未到不可挽回的地步,实现功能要紧,至于重构,还是以后再做吧!”这样一想,于是就会滋生得过且过的想法,等到代码开始变得一团糟时,重构已经变得无比困难了。此时需要的重构技巧,将百倍难于发现坏味道之初。


1



在我参加的前一个项目中,我们定义了一个处理OrderSet的Controller。刚刚开始开发时,对于OrderSet的操作并不多,主要是Search与Count操作。OrderSet分为WithDetails与WithoutDetail两种类型。


为了实现的简单,我们将这两种类型的操作都放在一个Controller中。随着操作的逐渐增多,这个Controller就变得越来越庞大,逐渐变得臃肿起来。
每当我需要调用或者修改Controller时,我都在想:“嗯,这个代码太糟糕了,什么时候给它重构一下。”想是这么想,却一直扮演着说话的巨人,行动的矮子。即使兴起这样的念头,又因为其他的工作将此念头浇灭。直到有一天,这个Controller的代码已经到了忍无可忍的地步,我和我的Pair终于达成一致意见,决定对此代码进行手术。
我们花费了近一天的时间对Controller以及相关的Repository进行了彻底的重构。重构前后的代码天差地别,我终于可以不用像吃了苍蝇那般看着代码恶心了。这种重构后体验到的愉悦简直无与伦比,最关键是结果令人满意,重构后的代码变得更可读,更简单,也更容易增加新的功能。


2



如今工作的项目,需要对遗留系统进行迁移。首要的任务是为此系统编写BDD测试,以期搭建迁移的测试保护网,并能够形成可读与可工作的测试用例文档。


在开始接触这个任务时,客户方的开发人员已经基本搭建好了初步的框架。当我们在面对不良代码时,第一个浮现在脑海中的念头是“重构”,然而考虑到时间因素,随之又强迫自己灭了这个念头,因为我们需要考虑项目的进度。我们总是在这样的取舍之中艰难前进,终于,在系统需要增加一个新消息的测试时,我强烈地感受到重构的迫在眉睫。即使代码有诸多破窗,修补了一扇,总强过听之任之。经过接近一天多的重构(当然还包括run tests以及build花费的时间),结果令人满意。
回顾这个过程,我发现在发现坏味道时,如果能及时地对代码进行重构,并保证重构的小步前进,并不会阻碍开发进度,相反还能够在一定程度改善代码质量,提高代码的可读性、可重用性以及可扩展性。
所谓“勿以善小而不为”,千万不要因为小重构对代码质量的影响微乎其微而轻视她,或者忽略她,又或者推迟到忍无可忍再想到重构。重构并非最后的救命稻草,而是随时保持我们正确前进的一把尺子。


3



说完了重构的重要性,让我再来粗略地介绍这个重构过程。


我们的测试程序主要针对Message的发送、接收与验证。业务的处理则由部署在JBoss上的应用来处理。我们需要设定期望的Message,在发送请求到远程系统后,接收返回的消息,并验证消息以及数据库是否符合我们的期望。重构的起因在于我们需要编写新的测试覆盖之前从未测试过的消息,其类型为SO08。
如果沿用之前的实现,我们就需要在测试步骤中增加MessageType的分支,根据消息类型对返回的消息进行检查。

检查的逻辑事实上已经被抽象为MessageChecker接口,并为各种类型的消息建立了不同的MessageChecker子类。MessageCheckFactory则是这些子类的工厂,负责根据类型创建对应的子类对象。这样的设计是完全合理的。
然而,问题出现在MessageReceiver,它提供了接收消息的方法,通过传入的消息类型、队列名称等参数,返回消息。这个返回值被定义为MessageReader。MessageReader正是问题的症结。
我一直强调的面向对象设计中一个重要概念就是所谓”对象的自治“,即对象的职责是自我完备的,它能够对自己拥有的数据负责,具备了“智能”处理的行为特征。

MessageReader违背了这一原则,它是愚笨的对象,仿佛“坐拥宝山而不知”的笨伯,虽然拥有消息的值,却不知道该如何处理这些消息。简而言之,它提供的方法只能对XML格式的消息进行读取,却不具有真正的业务行为。于是在测试步骤中,就产生了这样的代码(省略了部分实现代码):

private void checkPropagationQueueByName(String name, Queue queue, MessageType messageType) {
  MessageReader reader = messageReceiver.getMessageFor(messageType, identifier, queue);
  String messageText = reader.toString();
  if (messageType == SO05) {
      messageCheckFactory.checkerFor(messageType, getExpectedSO05ResponseFor(name), messageText);
  }
  if (messageType == SO07) {
      checkSO07Response(name, messageType, messageText)
  }
  if (messageType == SO08) {
  messageCheckFactory.checkerFor(messageType, getExpectedSO08ResponseFor(name), messageText)
      .checkResponse();
    }
}

不幸的是,这样的逻辑处理在其他测试步骤中同样存在。
我注意到,之所以要处理分支,是因为系统需要判断返回的消息是否符合期望,以实现测试目标。这个检查的逻辑根据不同的消息类型会有不同的处理逻辑(其中,主要逻辑则委派给由MessageCheckFactory创建的MessageChecker对象)。
从接口看,它们都需要接收返回的消息与期望的消息。以SO05为例,它需要返回的消息messageText,以及由getExpectedSO05ResponseFor(name)方法返回的期望的消息。对于SO07而言,实现方法稍显复杂,所以提取了一个私有方法checkSO07Response()来处理。

毫无疑问,我清楚地嗅到了代码的坏味道。重构势在必行。一方面,这个分支的处理是不合理的,随着消息类型的增多,这条分支语句会越来越长。关键是这种处理接收消息的逻辑不止存在这一处,这种没有封装的实现方式可能导致出现重复代码,违背了DRY原则。另一方面,则是对ExpectedMessage的处理。它分散在多个测试步骤中,有的放在AddUpdateCustomerSteps,有的则被提取到AbstractSteps类。从职责分配的角度看,测试步骤本身并不应该承担创建或获取ExpectedMessage的职责。

重构的目标就是MessageReceiver接口。我首先查看了MessageReceiver的实现类与调用者,发现其实现类只有一个,即DefaultMessageReceiver。调用者则非常多,调用的方法为getMessageFor()。
这个方法正是我要操刀手术的目标方法。我希望它能够返回ResponseMessage自治对象,而非MessageReader。这意味着我们既需要修改方法的签名,同时还需要修改实现。修改方法签名会影响到调用的依赖点。在依赖点较多的情况下,这种重构需要谨慎处理。


4



我认为,在重构时首先需要明确重构的目的是什么,然后还需要理清楚整个重构的步骤,最后有条不紊地实施重构。


显然,我们的目的是希望消除分支语句,并以统一的方式对各种类型的返回消息进行处理。根据对自治对象的分析,这意味着需要赋予ResponseMessage以行为,使得它自身就能够处理对ExpectedMessage的验证。


由于创建ExpectedMessage的逻辑是分散的,因此,我们需要首先对这部分功能进行重构。以getExpectedSO05ResponseFor(name)方法的重构为例。该方法的实现如下所示:

private MessageReader getExpectedSO05ResponseFor(String name) {
  MessageWriter writer;
  if (scenarioContext.hasExpectedMessage() &&
          scenarioContext.getExpectedMessage().getMessageType() == SO05) {
      writer = scenarioContext.getExpectedMessage();           
  } else {
      writer = transformerFactory.transformerFor(scenarioContext.getRequestMessage(), SO05)
              .forCustomer(name)
              .transform();
  }
  writer.selectBlock(SO05_MESSAGE_HEADER);
  writer.setFieldValue(MESSAGE_HEADER.USER_ID, null);
  writer.selectBlock(SO05_PROFILE);
  String identifier = storyContext.getCustomerIdentifier(name);
  writer.setFieldValue(PROFILE.PROFILE_ID, identifier);
  String customerVersion = scenarioContext.getCustomerVersion();
  writer.setFieldValue(PROFILE.USER_COUNT, customerVersion);
  if (writer.selectBlockIfExists(SO05_INDIVIDUAL)) {
      writer.setFieldValue(INDIVIDUAL_CUSTOMER_TYPE, null);
  }
  return messageFactory.readFor(SO05, writer.toString());
}

我们需要定义一个专门的对象来承担这一职责,因此,我引入了一个新对象ExpectedMessageFactory。通过采用Move Method手法(快捷键为F6,指IntelliJ下的快捷键,下同)可以完成这一重构。

若要通过IDE自动帮助我们完成这一重构,就首先需要将该方法定义为static方法。然而,观察该方法的实现,它调用了许多字段值,例如scenarioContext,transformFactory等。由于这些字段并非static的,一旦将方法设置为static,使用这些字段就会提示错误。因此,在进行Move Method重构之前,需要首先将该方法调用的字段提取为参数,即运用Extract Parameter重构手法(快捷键为Ctrl+Alt+P)。如果该方法还调用了其他方法,则需要分析了解这些方法存在多少依赖,从职责上看是否也需要转移?如果只有重构的目标方法调用了它,则可以将方法内联(快捷键位Ctrl+ALT+N)。

做好这些准备工作后,就可以移动方法了。所有的这些手法,IDE都提供了自动重构的工具,所以并不须要担心这样的重构会引入新的问题。转移了方法后,原来的依赖点就自动改为对静态方法的调用。由于我们还需要再将其修改为非静态方法,此时,我们需要手动地修改所有原来对静态方法的调用。同时,对于当初为了移动方便而提取出来的参数,在移动到新类后,还需要恢复其原有地位,即将这些参数再提取为字段(快捷键为Ctrl+ALT+F)。之所以要这样做,一方面可以减少方法的参数,使得方法变得更为简洁,另一方面也可以提高类的内聚性。在转移了方法后,我还对原方法进行了Extract Method重构(快捷键为Ctrl+ALT+M):

private MessageReader getExpectedSO05ResponseFor(String name) {
  MessageWriter writer = initializeExpectedMessage(name, SO05);
  setSO05MessageHeader(writer);
  setSO05Profile(name, writer);
  setSO05Individual(writer);
  return messageFactory.readFor(SO05, writer.toString());
}
private MessageWriter initializeExpectedMessage(String name, MessageType messageType) {
  MessageWriter messageWriter;
  if (scenarioContext.hasExpectedMessage() &&
          scenarioContext.getExpectedMessage().getMessageType() == messageType) {
      writer = scenarioContext.getExpectedMessage();           
  } else {
      writer = transformerFactory.transformerFor(scenarioContext.getRequestMessage(), messageType)
              .forCustomer(name)
              .transform();
  }
  return messageWriter;
}
private void setSO05MessageHeader(MessageWriter writer) {
  writer.selectBlock(SO05_MESSAGE_HEADER);
  writer.setFieldValue(MESSAGE_HEADER.USER_ID, null);
}
private void setSO05Profile(String name, MessageWriter writer) {
  writer.selectBlock(SO05_PROFILE);
  String identifier = storyContext.getCustomerIdentifier(name);
  writer.setFieldValue(PROFILE.PROFILE_ID, identifier);
  String customerVersion = scenarioContext.getCustomerVersion();
  writer.setFieldValue(PROFILE.USER_COUNT, customerVersion);
}
private void setSO05Individual(MessageWriter writer) {
  if (writer.selectBlockIfExists(SO05_INDIVIDUAL)) {
      writer.setFieldValue(INDIVIDUAL_CUSTOMER_TYPE, null);
  }
}

通过对方法的提取,一方面以能表达功能意图的方法名提高代码的可读性,另一方面还能通过这种重构发现可能重用的方法,例如上面代码片段中的initializeExpectedMessage(),就是在经过提取方法的重构后,才发现其实对于SO07消息而言,同样存在相同的初始化逻辑。

private MessageWriter getExpectedSO07WriterFor(String name) {
  MessageWriter writer = initializeExpectedMessage(name, SO07);
  setSO07Details(name, writer);
  setSO07Blocks(name, writer);
  return writer;
}


5



在完成对ExpectedMessage创建功能的重构后,接下来就可以考虑处理MessageReceiver了。


看起来,我们必须修改getMessageFor()方法的签名。一种稳妥的做法是暂时保留该方法,然后引入一个新方法,并在新方法中调用getMessageFor()方法。不过,这种方式需要我们手动地去修改所有依赖点;另一种做法则是先通过提取方法的方式,将原有getMessageFor()的所有实现提取到一个私有方法中,然后再直接利用修改方法签名的重构手法(快捷键为Ctrl+F6),直接修改getMessageFor()。这样做的好处是IDE工具可以直接帮助你修改所有的依赖点,同时还能够保留原有的实现。
为了更好地表达方法的意图,我还对该方法进行了更名重构(快捷键为Shift+F6),将其重命名为getResponseMessage()。由于方法的返回值发生了变化,所以依赖该方法的地方都会出现返回值类型不吻合的提示。在IntelliJ中,我们可以很容易地找到这些提示位置,并直接通过Alt+Enter根据工具给出的提示,来改变返回值类型。


改变了返回值类型并不意味着完事大吉,因为后面对该返回类型的调用,即前面提到的那段分支语句,仍然是不一致的。原来使用的是MessageReader对象,现在变成ResponseMessage对象了。这就需要我们手动地修改这些调用。当然,也有一种取巧的办法,就是将这些代码结合Extract Method与Move Method重构手法,再转移到我们引入的ResponseMessage中,因为在我们之前的分析中,已经明确这些分支判断逻辑应该封装到ResponseMessage对象。最终的重构结果为:

public abstract class ResponseMessage {
  public ResponseMessage(MessageReader messageReader) {
      this.messageReader = messageReader;
  }
  public void check(MessageReader expectedMessage) {
      messageCheckFactory.checkerFor(getMessageType(), expectedMessage, getMessageText()).checkResponse();
  }
  protected abstract MessageType getMessageType();
}
public class SO05ResponseMessage extends ResponseMessage {
  public SO05ResponseMessage(MessageReader messageReader) {
      super(messageReader);
  }
  @Override
  protected MessageType getMessageType() {
      return MessageType.SO05;
  }
}
public class DefaultMessageReceiver {
  public ResponseMessage getResponseMessage(MessageType type, String identifier, GCISQueue queue) {
      MessageReader messageReader = getMessageFor(type, identifier, queue);
      return createResponseMessage(type, messageReader, identifer);
  }
  private MessageReader getMessageFor(MessageType type, String identifier, GCISQueue queue) {
      MessageReader reader = getCachedMessageFor(type, identifier, queue);
      while (reader == null) {
          reader = getMessageFromQueueFor(type, identifer, queue);
      }
      return reader;
  }
  private ResponseMessage createResponseMessage(MessageType messageType, MessageReader messageReader, String identifer) {
      ResponseMessage message = null;
      switch (messageType) {
          case SO05:
              message = new SO05ResponseMessage(messageReader);
              break;
          case SO07:
              message = new SO07ResponseMessage(messageReader);
              break;
          case SO08:
              message = new SO08ResponseMessage(messageReader);
              break;
      }
      message.setMessageCheckFactory(messageCheckFactory);
      return message;
  }
}
//invoker
public class AddUpdateProductSystemCustomerSteps extends AbstractCustomerExpectationSteps {
  private void checkPropagationQueueByName(String name, Queue queue, MessageType messageType) {
      ResponseMessage responseMessage = messageReceiver.getMessageFor(messageType, identifier, queue);
  }
}


6



掌握重构的技巧并不难,关于在于你必须要有好的嗅觉,能够及时发现代码的坏味道。


然而,即使你拥有高超的重构技艺,如果未能养成随时重构的好习惯,又能如何?换言之,重构能力体现的是你的专业技能,而重构习惯体现的则是你的职业素养。你是否愿意为追求高质量的卓越代码而为之付出时间和精力呢?你能否以好的结果来说服客户尊重你的重构成果呢?我觉得,对卓越软件的追求,不仅限于自己,同时也需要将此理念灌输给客户,并使得客户愿意为之付费。从软件成本来看,这种对高质量软件的追求或许违背了短期利益,但绝对符合软件开发的长期利益。


所以,在下决心打磨代码质量之前,还是先找好重构这块磨刀石,并放到自己随时伸手可及的工具箱中吧。

相关文章
|
6月前
|
设计模式 算法
重构,避免重构误区
重构,避免重构误区
25 0
|
消息中间件 JavaScript 小程序
用1个月重构了同事写的烂代码,我总结出了15条重写烂代码的经验!
用1个月重构了同事写的烂代码,我总结出了15条重写烂代码的经验!
|
设计模式 SQL Java
有点狠有点猛,我用责任链模式重构了业务代码
文章开篇,抛出一个老生常谈的问题,学习设计模式有什么作用? 设计模式主要是为了应对代码的复杂性,让其满足开闭原则,提高代码的扩展性 另外,学习的设计模式 一定要在业务代码中落实,只有理论没有真正实施,是无法真正掌握并且灵活运用设计模式的 这篇文章主要说 责任链设计模式,认识此模式是在读 Mybatis 源码时, Interceptor 拦截器主要使用的就是责任链,当时读过后就留下了很深的印象(内心 OS:还能这样玩)
|
设计模式
重构·改善既有代码的设计.04之重构手法(下)完结
重构改善既有代码的设计完结篇,汇总了全部的重构手法。看看哪些手法对你的项目能有所帮助…
7359 2
重构·改善既有代码的设计.04之重构手法(下)完结
|
数据库 开发者
读书·技术 |《重构》· 重构的原则
读书·技术 |《重构》· 重构的原则
136 0
|
程序员
程序员如何做好代码重构?
代码重构重构就是在不改变软件系统外部行为的前提下,改善它的内部结构。重构不是重写,它们的区别你可以理解为,重构是修复代码,大框架不变。重写是扔掉原来的,重新设计框架。
161 0
程序员如何做好代码重构?
|
设计模式 新零售 供应链
一文教会你如何写复杂业务代码
这两天在看零售通商品域的代码。面对零售通如此复杂的业务场景,如何在架构和代码层面进行应对,是一个新课题。针对该命题,我进行了比较细致的思考和研究。结合实际的业务场景,我沉淀了一套“如何写复杂业务代码”的方法论,在此分享给大家。
28386 1
一文教会你如何写复杂业务代码
|
开发者 Python
一日一技:8行炫技代码,知识点多得不得了
一日一技:8行炫技代码,知识点多得不得了
251 0
一日一技:8行炫技代码,知识点多得不得了
|
Java 数据库 容器
项目重构,我是如何优化大量屎一样的 if else 代码的?
  目录:   if else策略模式1、首先抽象业务处理器2、将业务处理器和其支持处理的类型放到一个容器中,java里Map就是最常用的容器之一3、定义不同的处理器4、测试类   前段时间,我将公司系统中的批量审单的功能进行了重构,用到了java的并发编程进行异步化处理,数据库的乐观锁机制处理多线程并发更新数据。   其中批量审单的业务处理涉及到多种任务类型,对应不同的业务方法进行处理,比如转仓,转快递,添加赠品,删除赠品,拆分订单,批量驳回,批量作废等等,其中就用到了策略模式。
228 0
|
供应链 设计模式
一文教会你如何写复杂业务的代码
简单的介绍下业务背景,零售通是给线下小店供货的B2B模式,我们希望通过数字化重构传统供应链渠道,提升供应链效率,为新零售助力。阿里在中间是一个平台角色,提供的是Bsbc中的service的功能。
8287 0