重构 — 勿以善小而不为

天狗

　　重构最大的敌人不是技巧与能力，而是懒惰，或者说是态度。许多细小的重构看似无足轻重，例如方法重命名、提取方法。即使重构了，似乎对代码的结构也没有太大的影响，于是就决定淡然处之，心里想“事情还未到不可挽回的地步，实现功能要紧，至于重构，还是以后再做吧！”这样一想，于是就会滋生得过且过的想法，等到代码开始变得一团糟时，重构已经变得无比困难了。此时需要的重构技巧，将百倍难于发现坏味道之初。
　　在我参加的前一个项目中，我们定义了一个处理OrderSet的Controller。刚刚开始开发时，对于OrderSet的操作并不多，主要是Search与Count操作。OrderSet分为WithDetails与WithoutDetail两种类型。为了实现的简单，我们将这两种类型的操作都放在一个Controller中。随着操作的逐渐增多，这个Controller就变得越来越庞大，逐渐变得臃肿起来。每当我需要调用或者修改Controller时，我都在想：“嗯，这个代码太糟糕了，什么时候给它重构一下。”想是这么想，却一直扮演着说话的巨人，行动的矮子。即使兴起这样的念头，又因为其他的工作将此念头浇灭。直到有一天，这个Controller的代码已经到了忍无可忍的地步，我和我的Pair终于达成一致意见，决定对此代码进行手术。我们花费了近一天的时间对Controller以及相关的Repository进行了彻底的重构。重构前后的代码天差地别，我终于可以不用像吃了苍蝇那般看着代码恶心了。这种重构后体验到的愉悦简直无与伦比，最关键是结果令人满意，重构后的代码变得更可读，更简单，也更容易增加新的功能。

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

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

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

　　检查的逻辑事实上已经被抽象为MessageChecker接口，并为各种类型的消息建立了不同的MessageChecker子类。MessageCheckFactory则这些子类的工厂，负责根据类型创建对应的子类对象。这样的设计是完全合理的。然而，问题出现在MessageReceiver，它提供了接收消息的方法，通过传入的消息类型、队列名称等参数，返回消息。这个返回值被定义为MessageReader。

　　MessageReader正是问题的症结。我一直强调的面向对象设计中一个重要概念就是所谓”对象的自治“，即对象的职责是自我完备的，它能够对自己拥有的数据负责，具备了“智能”处理的行为特征。MessageReader违背了这一原则，它是愚笨的对象，仿佛“坐拥宝山而不知”的笨伯，虽然拥有消息的值，却不知道该如何处理这些消息。简而言之，它提供的方法只能对XML格式的消息进行读取，却不具有真正的业务行为。于是在测试步骤中，就产生了这样的代码（省略了部分实现代码）：

1. [color=#0000ff]private[/color] [color=#0000ff]void[/color][color=#000000] checkPropagationQueueByName(String name, Queue queue, MessageType messageType) {MessageReader reader [/color]=[color=#000000] messageReceiver.getMessageFor(messageType, identifier, queue);String messageText [/color]=[color=#000000] reader.toString();[/color][color=#0000ff]if[/color] (messageType ==[color=#000000] SO05) {messageCheckFactory.checkerFor(messageType, getExpectedSO05ResponseFor(name), messageText);}[/color][color=#0000ff]if[/color] (messageType ==[color=#000000] SO07) {checkSO07Response(name, messageType, messageText)}[/color][color=#0000ff]if[/color] (messageType ==[color=#000000] SO08) {messageCheckFactory.checkerFor(messageType, getExpectedSO08ResponseFor(name), messageText).checkResponse();}}[/color]  

复制代码

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

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

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

　　我以为，在重构时首先需要明确重构的目的是什么，然后还需要理清楚整个重构的步骤，最后有条不紊地实施重构。显然，我们的目的是希望消除分支语句，并以统一的方式对各种类型的返回消息进行处理。根据对自治对象的分析，这意味着需要赋予ResponseMessage以行为，使得它自身就能够处理对ExpectedMessage的验证。由于创建ExpectedMessage的逻辑是分散的，因此，我们需要首先对这部分功能进行重构。以getExpectedSO05ResponseFor(name)方法的重构为例。该方法的实现如下所示：

1. private MessageReader getExpectedSO05ResponseFor(String name) {
2. MessageWriter writer;
3. if (scenarioContext.hasExpectedMessage() &&
4. scenarioContext.getExpectedMessage().getMessageType() == SO05) {
5. writer = scenarioContext.getExpectedMessage();
6. } else {
7. writer = transformerFactory.transformerFor(scenarioContext.getRequestMessage(), SO05)
8. .forCustomer(name)
9. .transform();
10. }
11. writer.selectBlock(SO05_MESSAGE_HEADER);
12. writer.setFieldValue(MESSAGE_HEADER.USER_ID, null);
13. writer.selectBlock(SO05_PROFILE);
14. String identifier = storyContext.getCustomerIdentifier(name);
15. writer.setFieldValue(PROFILE.PROFILE_ID, identifier);
16. String customerVersion = scenarioContext.getCustomerVersion();
17. writer.setFieldValue(PROFILE.USER_COUNT, customerVersion);
18. if (writer.selectBlockIfExists(SO05_INDIVIDUAL)) {
19. writer.setFieldValue(INDIVIDUAL_CUSTOMER_TYPE, null);
20. }
21. return messageFactory.readFor(SO05, writer.toString());
22. }

复制代码

　我们需要定义一个专门的对象来承担这一职责，因此，我引入了一个新对象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）：

1. private MessageReader getExpectedSO05ResponseFor(String name) {
2. MessageWriter writer = initializeExpectedMessage(name, SO05);
3. setSO05MessageHeader(writer);
4. setSO05Profile(name, writer);
5. setSO05Individual(writer);
6. return messageFactory.readFor(SO05, writer.toString());
7. }
8. private MessageWriter initializeExpectedMessage(String name, MessageType messageType) {
9. MessageWriter messageWriter;
10. if (scenarioContext.hasExpectedMessage() &&
11. scenarioContext.getExpectedMessage().getMessageType() == messageType) {
12. writer = scenarioContext.getExpectedMessage();
13. } else {
14. writer = transformerFactory.transformerFor(scenarioContext.getRequestMessage(), messageType)
15. .forCustomer(name)
16. .transform();
17. }
18. return messageWriter;
19. }
20. private void setSO05MessageHeader(MessageWriter writer) {
21. writer.selectBlock(SO05_MESSAGE_HEADER);
22. writer.setFieldValue(MESSAGE_HEADER.USER_ID, null);
23. }
24. private void setSO05Profile(String name, MessageWriter writer) {
25. writer.selectBlock(SO05_PROFILE);
26. String identifier = storyContext.getCustomerIdentifier(name);
27. writer.setFieldValue(PROFILE.PROFILE_ID, identifier);
28. String customerVersion = scenarioContext.getCustomerVersion();
29. writer.setFieldValue(PROFILE.USER_COUNT, customerVersion);
30. }
31. private void setSO05Individual(MessageWriter writer) {
32. if (writer.selectBlockIfExists(SO05_INDIVIDUAL)) {
33. writer.setFieldValue(INDIVIDUAL_CUSTOMER_TYPE, null);
34. }
35. }

复制代码

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

1. private MessageWriter getExpectedSO07WriterFor(String name) {
2. MessageWriter writer = initializeExpectedMessage(name, SO07);
3. setSO07Details(name, writer);
4. setSO07Blocks(name, writer);
5. return writer;
6. }

复制代码

　在完成对ExpectedMessage创建功能的重构后，接下来就可以考虑处理MessageReceiver了。看起来，我们必须修改getMessageFor()方法的签名。一种稳妥的做法是暂时保留该方法，然后引入一个新方法，并在新方法中调用getMessageFor()方法。不过，这种方式需要我们手动地去修改所有依赖点；另一种做法则是先通过提取方法的方式，将原有getMessageFor()的所有实现提取到一个私有方法中，然后再直接利用修改方法签名的重构手法（快捷键为Ctrl+F6），直接修改getMessageFor()。这样做的好处是IDE工具可以直接帮助你修改所有的依赖点，同时还能够保留原有的实现。为了更好地表达方法的意图，我还对该方法进行了更名重构（快捷键为Shift+F6），将其重命名为getResponseMessage()。由于方法的返回值发生了变化，所以依赖该方法的地方都会出现返回值类型不吻合的提示。在IntelliJ中，我们可以很容易地找到这些提示位置，并直接通过Alt+Enter根据工具给出的提示，来改变返回值类型。
　　改变了返回值类型并不意味着完事大吉，因为后面对该返回类型的调用，即前面提到的那段分支语句，仍然是不一致的。原来使用的是MessageReader对象，现在变成ResponseMessage对象了。这就需要我们手动地修改这些调用。当然，也有一种取巧的办法，就是将这些代码结合Extract Method与Move Method重构手法，再转移到我们引入的ResponseMessage中，因为在我们之前的分析中，已经明确这些分支判断逻辑应该封装到ResponseMessage对象。最终的重构结果为：

1. public abstract class ResponseMessage {
2. public ResponseMessage(MessageReader messageReader) {
3. this.messageReader = messageReader;
4. }
5. public void check(MessageReader expectedMessage) {
6. messageCheckFactory.checkerFor(getMessageType(), expectedMessage, getMessageText()).checkResponse();
7. }
8. protected abstract MessageType getMessageType();
9. }
10. public class SO05ResponseMessage extends ResponseMessage {
11. public SO05ResponseMessage(MessageReader messageReader) {
12. super(messageReader);
13. }
14. @Override
15. protected MessageType getMessageType() {
16. return MessageType.SO05;
17. }
18. }
19. public class DefaultMessageReceiver {
20. public ResponseMessage getResponseMessage(MessageType type, String identifier, GCISQueue queue) {
21. MessageReader messageReader = getMessageFor(type, identifier, queue);
22. return createResponseMessage(type, messageReader, identifer);
23. }
24. private MessageReader getMessageFor(MessageType type, String identifier, GCISQueue queue) {
25. MessageReader reader = getCachedMessageFor(type, identifier, queue);
26. while (reader == null) {
27. reader = getMessageFromQueueFor(type, identifer, queue);
28. }
29. return reader;
30. }
31. private ResponseMessage createResponseMessage(MessageType messageType, MessageReader messageReader, String identifer) {
32. ResponseMessage message = null;
33. switch (messageType) {
34. case SO05:
35. message = new SO05ResponseMessage(messageReader);
36. break;
37. case SO07:
38. message = new SO07ResponseMessage(messageReader);
39. break;
40. case SO08:
41. message = new SO08ResponseMessage(messageReader);
42. break;
43. }
44. message.setMessageCheckFactory(messageCheckFactory);
45. return message;
46. }
47. }
48. //调用者
49. public class AddUpdateProductSystemCustomerSteps extends AbstractCustomerExpectationSteps {
50. private void checkPropagationQueueByName(String name, Queue queue, MessageType messageType) {
51. ResponseMessage responseMessage = messageReceiver.getMessageFor(messageType, identifier, queue);
52. }
53. }

复制代码

　　掌握重构的技巧并不难，关于在于你必须要有好的嗅觉，能够及时发现代码的坏味道。然而，即使你拥有高超的重构技艺，如果未能养成随时重构的好习惯，又能如何？换言之，重构能力体现的是你的专业技能，而重构习惯体现的则是你的职业素养。你是否愿意为追求高质量的卓越代码而为之付出时间和精力呢？你能否以好的结果来说服客户尊重你的重构成果呢？我觉得，对卓越软件的追求，不仅限于自己，同时也需要将此理念灌输给客户，并使得客户愿意为之付费。从软件成本来看，这种对高质量软件的追求或许违背了短期利益，但绝对符合软件开发的长期利益。 　　所以，在下决心打磨代码质量之前，还是先找好重构这块磨刀石，并放到自己随时伸手可及的工具箱中吧。

asdstar

学习啦 貌似很少重构 没时间

赖氵

:lol:lol:lol