记一次代码重构
單一職責
功能單一
功能單一是SRP最基本要求，也就是你一個類的功能職責要單一，這樣內聚性才高。
比如，下面這個參數類，是用來查詢網站Buyer信息的，按照SRP，里面就應該放置查詢相關的Field就好了。
@Data public class BuyerInfoParam {// Required Paramprivate Long buyerCompanyId;private Long buyerAccountId;private Long callerCompanyId;private Long callerAccountId;private String tenantId;private String bizCode;private String channel; //這個Channel在查詢中不起任何作用，不應該放在這里 }可是呢？ 事實并不是這樣，下面的三個參數其實查詢時根本用不到，而是在組裝查詢結果的時候用到，這給我閱讀代碼帶來了很大的困惑，因為我一直以為這個channel（客戶來源渠道）是一個查詢需要的一個重要信息。
那么如果和查詢無關，為什么要把它放到查詢param里面呢，問了才知道，只是為了組裝查詢結果時拿到數據而已。
所以我重構的時候，果斷把查詢沒用到的參數從BuyerInfoParam中移除了，這樣就消除了理解上的歧義。
Tips：不要為了圖方便，而破壞SOLID原則，方便的后果就是代碼腐化，看不懂，往后要付出的代價更高。
功能內聚
在類的職責單一基礎之上，我們還要識別出是不是有功能相似的類或者組件，如果有，是不是要整合起來，而不要讓功能類似的代碼散落在多處。
比如，代碼中我們有一個TenantContext，而build這個Context統一是在ContextPreInterceptor中做的，其中Operator的值一開始只有crmId，但是隨著業務的變化operator的值在不同的場景值會不一樣，可能是aliId，也可能是accountId。
這樣就需要其它id要轉換成crmId的工作，重構前這個轉換工作是分散在多個地方，不滿足SRP。
//在BaseMtopServiceImpl中有crmId轉換的邏輯public String getCrmUserId(Long userId){AccountInfoDO accountInfoDO = accountQueryTunnel.getAccountDetailByAccountId(userId.toString(), AccountTypeEnum.INTL.getType(), false);if(accountInfoDO != null){return accountInfoDO.getCrmUserId();}return StringUtils.EMPTY;}//在ContextUtilServiceImpl中有crmId轉換的邏輯public String getCrmUserIdByMemberSeq(String memberSeq) {if(StringUtil.isBlank(memberSeq)){return null;}MemberMappingDO mappingDO = memberMappingQueryTunnel.getMappingByAccountId(Long.valueOf(memberSeq));if(mappingDO == null || mappingDO.getAliId() == null){return null;}}重構的做法是將build context的邏輯，包括前序的crmId的轉換邏輯，全部收攏到ContextPreInterceptor，因為它的職責就是build context，這樣代碼的內聚性，可復用性和可讀性都會好很多。
@Overridepublic void preIntercept(Command command) {...String crmUserId = getCrmUserId(command);if(StringUtil.isBlank(crmUserId)){throw new BizException(ErrorCode.BIZ_ERROR_USER_ID_NULL, "can not find crmUserId with "+command.getOperater());}...}//將crmId的轉換邏輯收攏至此private String getCrmUserId(Command command) {if(command.getOperatorIdType() == OperatorIdType.CRM_ID){return command.getOperater();}else if(command.getOperatorIdType() == OperatorIdType.ACCOUNT_ID){MemberMappingDO mappingDO = memberMappingQueryTunnel.getMappingByAccountId(Long.valueOf(command.getOperater()));if(mappingDO == null || mappingDO.getAliId() == null){return null;}return fetchByAliId(mappingDO.getAliId().toString());}else if(command.getOperatorIdType() == OperatorIdType.ALI_ID){return fetchByAliId(command.getOperater());}return null;}開閉原則
OCP是OO非常重要的原則，遵循OCP可以極大的提升我們代碼的擴展性，要想寫出好的OCP代碼，前提是要熟練掌握常用的設計模式，常用的有助于OCP的設計模式有Abstract Factory，Template，Strategy，Chain of Responsibility, Obeserver, Decorator等
關于OCP的重構需要注意兩點：
- 不要濫用設計模式：不要有了錘子就到處亂錘，不恰當的使用不解決問題不說，還會增加復雜度。
- 要敢于重構：很多的功能點一開始都不需要擴展的，但是隨著業務的發展，會變得需要擴展，此時就需要果敢一點，該重構重構，該上設計模式，上，不能等，不能將就！
這里主要以Template和Chain of Responsibility為例，來介紹關于OCP的重構。
模板方法
比如，在處理Leads的時候，針對不同的Leads來源，其處理可能稍有不同，所以我重構的時候，覺得模板方法是比較好的選項。
因為對于不同的Leads來源，只有在線索已存在，但沒創建過客戶的情況下，處理邏輯不同，其它邏輯都可以共用，那么把processRepeatedLeadsWithNoCustomer設置為abstract就好了。
Tips：在使用設計模式的時候，最好能在類的命名上體現這個設計模式，這樣看代碼的人會更容易理解。
public abstract class AbstractLeadsProcessTemplate implements LeadsProcessServiceI {...@Overridepublic void processLeads(IcbuLeadsE icbuLeadsE) {IcbuLeadsE existingLeads = icbuLeadsE.checkRepeat();//1.新線索if(existingLeads == null){processBrandNewLeads(icbuLeadsE);}// 2. 線索已經存在，但是沒有創建過客戶else if(existingLeads.isCustomerNotCreated()){processRepeatedLeadsWithNoCustomer(existingLeads, icbuLeadsE);}//3. 線索已經存在，且創建過客戶，嘗試撿回私海else{processRepeatedLeadsAndCustomer(existingLeads, icbuLeadsE);}}/*** 處理線索已存在，客戶未創建* @param existingLeads 系統中已存在的Leads* @param comingLeads 新來的Leads*/public abstract void processRepeatedLeadsWithNoCustomer(IcbuLeadsE existingLeads, IcbuLeadsE comingLeads);責任鏈
在我們的營銷系統中，有一個EDM（Email Direct Marketing）的功能，在發送郵件之前，我們要根據規則過濾掉一些客戶，比如沒有郵箱地址，沒有訂閱關系，3天內重復發送等等，而且這個規則隨著業務的變化，很可能會增加或減少。
這里用if-else當然也能解決問題，但很明顯不滿足OCP，如果用一個Pipeline的模式，其擴展性就會好很多，類似于Servlet API中的FilterChain，增加、刪除Filter都很靈活。
FilterChain filterChain = FilterChainFactory.buildFilterChain(NoEmailAddressFilter.class, EmailUnsubscribeFilter.class, EmailThreeDayNotRepeatFilter.class);//具體的Filter public class NoEmailAddressFilter implements Filter {@Overridepublic void doFilter(Object context, FilterInvoker nextFilter) {Map<String, Object> contextMap = (Map<String, Object>)context;String email = ConvertUtils.convertParamType(contextMap.get("email"), String.class);if(StringUtils.isBlank(email)){contextMap.put("success", false);return;}nextFilter.invoke(context);} }SOFA框架
這次代碼重構中發現，對于框架概念的誤用，也是造成代碼混亂的原因之一，在SOFA框架中我們明確定義了module，package和class的scope和功能，但是在實施過程中，還是出現了在層次歸屬，命名和使用上的一些混亂，特別是Convertor，Assembler和擴展點。
Convertor
在SOFA中有三類主要的對象：
而Convertor在上面三個對象之間的轉換起到了至關重要的作用，然而Convertor里面的邏輯應該是簡單的，大部分都是setter/getter, 如果屬性重復度很高的話，也可以使用BeanUtils.copyProperties讓代碼變得更簡潔。
但事實情況是，現在系統中很多的Convertor邏輯并沒有在Convertor里面。
比如將詢盤數據convert成LeadsE，處理類是LeadsBuildStrategy，這個命名是不合適的。
所以我將這段邏輯重構到ICBULeadsConvertor也等于是在清晰的告訴看代碼的人，這里是做了一個Client數據到LeadsEntity的轉換，僅此而已。
Assembler
Assembler在SOFA框架中的定義是組裝參數使用的，所以看到Assembler我就很清楚這個類是用來組裝參數的。
例如，組裝OpenSearch的查詢條件，原來用的是擴展點CustomerRepeatRuleExtPt
但是RuleExt這個概念，只有在不同業務場景下的業務擴展才需要用到，所以這種命名和使用是不恰當的，組裝參數直接用RepeatCheckConditionAssembler就好了。
重構前：
List<RepeatConditionDO> repeatConditions = ruleExecutor.execute(CustomerRepeatRuleExtPt.class, repeatExt -> repeatExt.getRepeatCondition(queryField));重構后：
RepeatConditionDO repeatConditionDO = repeatCheckConditionAssembler.assembleCustomerRepeatCheckCondition(repeatCheckParam);擴展點
擴展點是我SOFA框架中針對不同業務或租戶的一種擴展機制，現在代碼中有一些使用，但是因為我們目前只有ICBU的場景，所以基本上所有的擴展點只有一個擴展實現。
如果這樣的話，我建議先不要提前抽出來擴展點，等有多業務場景的時候，再去重構。
例如OppotunityDistributeRuleExtPt、OpportunityOrgChangeRuleExtPt、LeadsCreateCustomerRuleExtPt、CustomerNoteAppendRuleExtPt等等，這樣的case有很多。
如果是業務內的擴展，使用Strategy Pattern就好了。
Tips：簡單一點，敏捷一點，不要太多的“提前設計和規劃”，等變化來了再重構，Keep it Simple!
領域建模
領域建模的核心，是在深入理解業務的基礎上，進行合理的領域抽象，將重要的業務知識、領域概念用通用語言（Ubiquitous Langua）的方式，統一的在PRD，模型和代碼中進行顯性化的表達，從而提升代碼的可讀性和可維護性。
所以能否合理的抽象出Value Object，Domain Entity，領域行為和領域服務，將成為DDD運用是否得當的關鍵。
Tips：錯誤的抽象，隨心而欲的亂抽象，還不如不要抽象。
領域對象
領域對象主要包括ValueObject和Entity，二者都是在表達一個重要的領域概念，其區別在于ValueObject是immutable的，而Entity不是，它需要有唯一的id做標識。
在進行領域對象抽取的時候，要注意以下兩點：
1、不要把重要的領域概念遺棄在Domain外面：
比如這次重構的主要業務——Leads引入，原來的Leads Entity形同虛設，業務邏輯都散落在外面，像Leads判重，Leads生成客戶等業務知識都沒有在Entity上體現出來，所以這種建模就是不合理的，也違背了DDD的初衷。
2、不要把非領域概念的對象強加到Domain中：
比如RepeatQueryFieldV只是一個Search的查詢參數，不應該是一個ValueObject，更合適的做法是定義成一個RepeatCheckParam放到Infrastructure層去，這樣理解起來更方便。
分析領域
客戶通的Leads來源很大部分來自于網站的詢盤（inquiry）聯系人，所以對于新的詢盤，我們當成新的Leads來處理。但是網站那邊的詢盤聯系人修改呢，這個很明顯是Contact域的事情，如果你和Leads揉在一起，會導致混亂。
public static LeadsEntryEvent ofAction(String action, LeadsE leads) {LeadsEntryEvent event = null;LeadsEntryAction entryAction = LeadsEntryAction.of(action);if (null != entryAction) {switch (entryAction) {case ADD:event = new LeadsAddEvent(leads);break;case UPDATE://TODO: This is not Leads, 是聯系人，不要放在Leads里處理event = new LeadsUpdateEvent(leads);break;case DELETE://TODO: This is not Leads, 是聯系人，不要放在Leads里處理event = new LeadsDeleteEvent(leads);break;case ASSIGN://TODO: This is not Leads, 不要放在Leads里處理event = new LeadsAssignEvent(leads);break;case SYNC://TODO: to deleteevent = new LeadsSyncEvent(leads);break;case IM_ADD:event = new LeadsImChannelAddEvent(leads);break;case MC_ADD:event = new LeadsMcChannelAddEvent(leads);break;default:很多的行為，放在多個Domain上都可以做，這個時候就要仔細分析，多動動頭腦，想想哪個Domain是最合適的Domain，而不是戴著耳機，瞧著代碼，實現業務功能，完事走人，留下滿地的衛生紙！
領域行為 vs. 領域服務
區分領域行為和領域服務，可以參考下面的劃分：
- 領域行為：是我這個Entity范疇之內的，添加到Entity身上，千萬不要不假思索的就抽成一個DomainService，這樣Entity很容易被架空。
- 領域服務：不是一個Entity能handle了的行為，可以考慮抽到更上層的DomainService中去。
因此，對于增加新Leads這個動作來說，直覺上應該是屬于LeadsEntity的，但是仔細分析一下，我們會發現在增加新Leads的同時，還要創建客戶、聯系人。如果有分配需要的話，還要將機會分配到業務員的私海。
這么多的邏輯，這么多Entity的交互，如果再放到LeadsEntity就不合適了，所以重構的時候，我抽象出LeadsProcessService這個DomainService，這樣在表達上會更加清晰，同事擴展起來也更方便。
@Overridepublic void processLeads(IcbuLeadsE icbuLeadsE) {IcbuLeadsE existingLeads = icbuLeadsE.checkRepeat();//1.新線索if(existingLeads == null){processBrandNewLeads(icbuLeadsE);}// 2. 線索已經存在，但是沒有創建過客戶else if(existingLeads.isCustomerNotCreated()){processRepeatedLeadsWithNoCustomer(existingLeads, icbuLeadsE);}//3. 線索已經存在，且創建過客戶，嘗試撿回私海else{processRepeatedLeadsAndCustomer(existingLeads, icbuLeadsE);}}統一語言
PRD中的語言，我們平時溝通的語言，模型中的語言和我們的代碼中的語言要保持一致，如果不一致，或者缺失都會極大的增加我們的代碼理解成本，使得代碼維護變得困難。
比如客戶判重，聯系人判重都是非常重要的領域概念，但是在我們領域模型上并沒有被體現，應該將其凸顯出來：
/*** 客戶判重** @return 如果有重復的客戶，返回其customerId, 否則返回null*/public String checkRepeat() {RepeatCheckParam repeatCheckParam = RepeatCheckParam.ofCompanyName(companyName);...}/*** 聯系人判重，主要通過email來判重* @return 如果有重復的聯系人，返回其contactId, 否則返回null*/public String checkRepeat(){if(email == null || email.size() == 0){return null;}//只檢查第一個email,因為icbu業務線中一個聯系人只有一個emailRepeatCheckParam repeatCheckParam = RepeatCheckParam.ofContactEmail(email.iterator().next());...}在系統中目前還有checkConflict表示判重，這個需要團隊達成一致，如果大家都同意用checkRepeat表示判重，那以后就統一用checkRepeat。
這地方不要糾結翻譯的準確性什么的，就像公海這個概念我們用的是PublicSea，這是一個很明顯的chinglish，但是大家讀起來順口，也容易理解，我覺得對于中國程序員來說就比SharedTerritory要好。
業務語義顯性化
還是那句話，代碼是寫給人讀的，機器能執行的代碼誰都可以寫，寫出人能讀懂的代碼才是NB。
下面以兩個重構案例來說明什么是業務語義顯性化，大家自己體會一下差別：
1、判斷Leads是否創建過客戶，其邏輯是看Leads是否有CustomerId
重構前：
if (StringUtil.isBlank(existLeads.getCustomerId())重構后：
if(existingLeads.isCustomerNotCreated())Tips：雖然只是一行代碼，但是卻是編程認知的差別。
2、判斷Customer是否在自己的私海
重構前：
public boolean checkPrivate(CustomerE customer) {IcbuCustomerE icbuCustomer = (IcbuCustomerE)customer;return !PvgContext.getCrmUserId().equals(NIL_VALUE)&& icbuCustomer.getCustomerGroup() != CustomerGroup.AliCrmCustomerGroup.CANCEL_GROUP;}重構后：
/*** 判斷是否可以被撿入私海* @return*/public boolean isAvailableForPrivateSea(){//如果不是業務員操作，不能撿入私海if(StringUtil.isBlank(PvgContext.getCrmUserId())){return false;}//如果是"刪除分組"的客戶，不撿入私海，放到公海if(this.getCustomerGroup() == CustomerGroup.AliCrmCustomerGroup.CANCEL_GROUP){return false;}return true;}性能優化
B類的服務壓力相對沒有那么大，性能往往不是瓶頸，但是并不意味著可以隨便揮霍。
比如，Leads更新操作中，發現一個更新，只需要更新一個字段，但是使用的是全字段更新（有幾十個字段），明顯的浪費資源，所以新增了一個單字段更新的SQL。
//重構前用這個<update id="update" parameterType="CrmLeadsDO">UPDATE crm_leads SET GMT_MODIFIED = now()<if test="modifier != null">, modifier = #{modifier}</if><if test="isDeleted != null">, is_deleted = #{isDeleted}</if><if test="customerId != null">, customer_id = #{customerId}</if><if test="referenceUserId != null">, reference_user_id = #{referenceUserId}</if>... 此處省略N多字段<if test="bizCode != null">, biz_code = #{bizCode}</if>where id = #{id} and tenant_id = #{tenantId} and IS_DELETED = 'n'</update>//重構后用這個<update id="updateCustomerId" parameterType="CrmLeadsDO">UPDATE crm_leads SET GMT_MODIFIED = now()<if test="customerId != null">, customer_id = #{customerId}</if>where id = #{id} and tenant_id = #{tenantId} and IS_DELETED = 'n'</update>Tips：性能優化要扣細節，不要一提到性能，就上ThreadPool。
測試代碼
看了大家的測試代碼，大部分都寫的很亂，沒有結構化。
實際上，測試代碼很容易結構化的，就是三個步驟：
下面是我寫的創建Leads，但是客戶沒有創建過的測試用例：
@Testpublic void testLeadsExistWithNoCustomer(){//1. PrepareIcbuLeadsAddCmd cmd = getIcbuIMLeadsAddCmd();String tenantId = "cn_center_10002404";LeadsE leadsE = leadsRepository.getLeads(tenantId, cmd.getContactId());if(leadsE != null) {leadsE.setCustomerId(null);leadsRepository.updateCustomerId(leadsE);}//2. ExecutecommandBus.send(cmd);//3. CheckleadsE = leadsRepository.getLeads(tenantId, cmd.getContactId());Assert.assertEquals(leadsE.getCustomerId(), "179001");}測試代碼允許有重復，因為這樣可以做到更好的隔離，不至于改了一個數據，影響到其他的測試用例。
Tips：PandoraBoot啟動很慢，如果不是全mock的話，建議使用我寫的TestContainer，這樣可以提效很多。
總結
- 上一篇： 前端面试题整理
- 下一篇： DataBinding基础使用二