記一次程式碼重構
單一職責
功能單一
功能單一是SRP最基本要求，也就是你一個類的功能職責要單一，這樣內聚性才高。
比如，下面這個引數類，是用來查詢網站Buyer資訊的，按照SRP，裡面就應該放置查詢相關的Field就好了。
@Data
public class BuyerInfoParam {
// Required Param
private 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，這樣程式碼的內聚性，可複用性和可讀性都會好很多。
@Override
public 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 {
...
@Override
public 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 {
@Override
public 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中有三類主要的物件：
- ClientObject: 也就是二方庫中的資料物件，主要承擔DTO的職責。
- Entity/ValueObject: 也就是既有屬性又有行為的領域實體。
- DataObeject：是用來獲取資料用的，主要是DAO使用。
而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 delete
event = 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，這樣在表達上會更加清晰，同事擴充套件起來也更方便。
@Override
public 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業務線中一個聯絡人只有一個email
RepeatCheckParam 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。
測試程式碼
看了大家的測試程式碼，大部分都寫的很亂，沒有結構化。
實際上，測試程式碼很容易結構化的，就是三個步驟：
- Prepare：準備資料，包括準備請求引數和資料清理
- Execute：執行要測試的程式碼
- Check：校驗執行結果
下面是我寫的建立Leads，但是客戶沒有建立過的測試用例：
@Test
public void testLeadsExistWithNoCustomer(){
//1. Prepare
IcbuLeadsAddCmd cmd = getIcbuIMLeadsAddCmd();
String tenantId = "cn_center_10002404";
LeadsE leadsE = leadsRepository.getLeads(tenantId, cmd.getContactId());
if(leadsE != null) {
leadsE.setCustomerId(null);
leadsRepository.updateCustomerId(leadsE);
}
//2. Execute
commandBus.send(cmd);
//3. Check
leadsE = leadsRepository.getLeads(tenantId, cmd.getContactId());
Assert.assertEquals(leadsE.getCustomerId(), "179001");
}測試程式碼允許有重複，因為這樣可以做到更好的隔離，不至於改了一個資料，影響到其他的測試用例。
Tips：PandoraBoot啟動很慢，如果不是全mock的話，建議使用我寫的TestContainer，這樣可以提效很多。
相關文章
- 記一次百萬行WPF專案程式碼的重構記錄
- 日記9(程式碼重構)
- 記一次專案重構
- 程式碼重構--大話重構
- 程式碼重構
- 程式碼重構之法——方法重構分析
- 記一次 Vue 專案的重構Vue
- “硬核”程式碼重構
- 重構 PHP 程式碼PHP
- PHP程式碼重構PHP
- 程式碼重構（四）
- 重構：改善既有程式碼的設計（第二版讀書筆記） - 重構、壞程式碼、寫好程式碼筆記
- 《重構，改善既有程式碼的設計》筆記筆記
- 記一次重構，策略模式替換if else模式
- 程式碼重構技巧（二）
- 談談程式碼重構
- 【讀程式碼重構有感】
- 重構：程式碼異味
- 程式碼重構：類重構的 8 個小技巧
- .NET重構—單元測試的程式碼重構
- 《重構:改善既有程式碼的設計》讀書筆記筆記
- 《重構-改善既有程式碼的設計》讀書筆記筆記
- 找出那些程式碼裡的壞味道吧——《重構》筆記筆記
- 《重構，改善既有程式碼的設計》讀書筆記筆記
- 程式碼重構那些事兒
- 重構你的javascript程式碼JavaScript
- 重構 - 程式碼優化技巧優化
- 重構 - 程式碼整潔之道
- 重構之提煉程式碼
- 重構改善既有的程式碼設計（重構原則）
- 程式碼重構：函式重構的 7 個小技巧函式
- 《重構：改善既有程式碼的設計》讀書筆記（一）筆記
- 《重構 改善既有程式碼的設計》 讀書筆記（十五）筆記
- 程式碼重構與單元測試——“提取方法”重構（三）
- 重構程式碼（應如寫詩）
- 程式碼的壞味道和重構
- 高效重構 C++ 程式碼（下）C++
- 高效重構 C++ 程式碼（上）C++