Single responsibility
Single function
Single function is the most basic requirement of SRP, that is to say, the function and responsibility of your class should be single, so that cohesion is high.
For example, the following parameter class is used to query website Buyer information, according to SRP, it should be placed in the query 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; //This Channel doesn't work in queries and shouldn't be put here. }
But what about it? This is not the case. The following three parameters are not used at all when querying, but when assembling the query results. This brings me a lot of confusion when reading the code, because I always think that this channel (customer source channel) is an important information for querying.
So if it has nothing to do with the query, why put it in the query param? Just ask to get the data when assembling the query results.
So when I refactored, I decisively removed unused parameters from BuyerInfoParam, which eliminated the ambiguity of understanding.
Tips: Don't break the SOLID principle for convenience. The consequence of convenience is code corruption, and you don't understand it. It will cost more in the future.
Functional Cohesion
On the basis of the single responsibility of classes, we also need to identify whether there are classes or components with similar functions, and if so, whether they should be integrated rather than scattering code with similar functions.
For example, we have a TenantContext in the code, and build this Context unification is done in ContextPreInterceptor, where the Operator's value is only crmId at first, but the value of the Operator varies in different scenarios as the business changes, maybe aliId or accountId.
This requires other IDS to be converted to crmId. Before refactoring, this conversion work is scattered in many places and does not satisfy SRP.
//Logic for crmId transformation in BaseMtopService Impl public String getCrmUserId(Long userId){ AccountInfoDO accountInfoDO = accountQueryTunnel.getAccountDetailByAccountId(userId.toString(), AccountTypeEnum.INTL.getType(), false); if(accountInfoDO != null){ return accountInfoDO.getCrmUserId(); } return StringUtils.EMPTY; } //Logic for crmId transformation in ContextUtilService Impl 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; } }
The refactoring approach is to bring the logic of the build context, including the transformation logic of the preceding crmId, into the ContextPreInterceptor, because its responsibility is to build context, so that the cohesion, reusability and readability of the code will be much better.
@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()); } ... } //Close the transformation logic of crmId to this point 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; }
Open and Close Principle
OCP is a very important principle of OO. Following OCP can greatly improve the extensibility of our code. To write good OCP code, the premise is to master commonly used design patterns. The commonly used design patterns for OCP are Abstract Factory, Template, Strategy, Chain of Responsibility, Obeserver, Decorator, etc.
Two points should be paid attention to when reconstructing OCP:
- Don't abuse design patterns: don't hammer everywhere without a hammer, improper use will increase complexity if it doesn't solve the problem.
- To dare to rebuild: many functional points do not need to be expanded at the beginning, but with the development of business, it will become necessary to expand, at this time we need to be bold, the rebuilding and reconstruction, the design pattern, can not wait, can not be done!
Here, Template and Chain of Responsibility are taken as examples to introduce the reconfiguration of OCP.
Template method
For example, when dealing with Leads, the processing may be slightly different for different sources of Leads, so when I refactoring, I think template method is a better option.
Because for different sources of Leads, only if the thread already exists but no customer has been created, the processing logic is different and other logic can be shared, so it's better to set the process Repeated Leads Within No Customer to abstract.
Tips: When using design patterns, it's better to embody the design patterns in the naming of classes so that people who look at the code can understand them more easily.
public abstract class AbstractLeadsProcessTemplate implements LeadsProcessServiceI { ... @Override public void processLeads(IcbuLeadsE icbuLeadsE) { IcbuLeadsE existingLeads = icbuLeadsE.checkRepeat(); //1. New clues if(existingLeads == null){ processBrandNewLeads(icbuLeadsE); } // 2 Clues already exist, but no customers have been created. else if(existingLeads.isCustomerNotCreated()){ processRepeatedLeadsWithNoCustomer(existingLeads, icbuLeadsE); } //3 Clues already exist and customers have been created to try to retrieve private seas. else{ processRepeatedLeadsAndCustomer(existingLeads, icbuLeadsE); } } /** * Processing thread already exists, customer not created * @param existingLeads Existing Leads in the System * @param comingLeads New Leads */ public abstract void processRepeatedLeadsWithNoCustomer(IcbuLeadsE existingLeads, IcbuLeadsE comingLeads);
Chain of responsibility
In our marketing system, there is an EDM (Email Direct Marketing) function. Before sending an email, we need to filter out some customers according to the rules, such as no email address, no subscription relationship, repeated sending within three days, and so on. And this rule is likely to increase or decrease with the change of business.
if-else can also solve the problem, but it is obviously not satisfied with OCP. If you use a Pipeline model, its scalability will be much better, similar to FilterChain in Servlet API, adding and deleting Filters are flexible.
FilterChain filterChain = FilterChainFactory.buildFilterChain( NoEmailAddressFilter.class, EmailUnsubscribeFilter.class, EmailThreeDayNotRepeatFilter.class); //Specific 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 Framework
This code refactoring found that the misuse of framework concepts is also one of the causes of code confusion. SOFA Framework We clearly define the scope and functions of module s, package s and class es, but in the implementation process, there are still some confusion in hierarchical attribution, naming and use, especially Convertor, Assembler and extension points.
Convertor
There are three main types of objects in SOFA:
- ClientObject: A data object in a two-party library, which is mainly responsible for DTO.
- Entity/ValueObject: A domain entity that has both attributes and behaviors.
- DataObeject: It's used to get data, mainly DAO.
Convertor plays a crucial role in the transformation between the above three objects. However, the logic in Convertor should be simple, most of which are setter/getter. BeanUtils.copyProperties can also be used to make the code more concise if the attribute repetition is high.
But the fact is that a lot of Convertor logic in the system is not in Convertor.
For example, convert ing inquiry data into LeadsE, the processing class is LeadsBuildStrategy, which is inappropriate.
So I refactored this logic to ICBU Leads Convertor, which is also a clear way to tell people who are looking at the code. Here is a conversion from Client data to Leads Entity, that's all.
Assembler
Assembler's definition in the SOFA framework is used for assembling parameters, so when I see Assembler, I know that this class is used for assembling parameters.
For example, the query condition for assembling OpenSearch was originally used with the extension point Customer Repeat Rule ExtPt
However, the concept of RuleExt is only needed for business extensions in different business scenarios, so this naming and use is inappropriate. Just use Repeat Check ConditionAssembler directly for assembly parameters.
Before reconstruction:
List<RepeatConditionDO> repeatConditions = ruleExecutor.execute(CustomerRepeatRuleExtPt.class, repeatExt -> repeatExt.getRepeatCondition(queryField));
After reconstruction:
RepeatConditionDO repeatConditionDO = repeatCheckConditionAssembler.assembleCustomerRepeatCheckCondition(repeatCheckParam);
Extension Point
Extension Point It is an extension mechanism in my SOFA framework for different businesses or tenants. Now there are some uses in the code, but because we only have ICBU scenarios at present, basically all extension points have only one extension implementation.
If so, I recommend not pulling out extension points in advance, and then refactoring when there are multiple business scenarios.
For example, Oppotunity Distributed Rule ExtPt, Opportunity OrgChangeRule ExtPt, Leads Create Customer Rule ExtPt, Customer Note AppendRule ExtPt, etc., there are many such case s.
If it's an extension within the business, use Strategy Pattern s.
Tips: Simple, agile, don't "design and plan ahead" too much, and then refactor when changes come, Keep it Simple!
Domain Modeling
The core of domain modeling is to make rational domain abstraction on the basis of in-depth understanding of business, unify important business knowledge and domain concepts in the way of Ubiquitous Langua, and express them explicitly in PRD, model and code, so as to improve the readability and maintainability of code.
So whether we can abstract Value Object, Domain Entity, Domain Behavior and Domain Services reasonably will be the key to the proper use of DDD.
Tips: It's better not to be abstract than wrong abstraction and random abstraction.
Domain object
Domain objects mainly include ValueObject and Entity, both of which express an important domain concept. The difference is that ValueObject is immutable, but Entity is not. It needs a unique id to identify.
When extracting domain objects, we should pay attention to the following two points:
1. Don't leave important domain concepts outside Domain:
For example, the introduction of Leads, the main business of this restructuring, the original Leads Entity is virtual, business logic is scattered outside, such as Leads weighing, Leads generating customers and other business knowledge are not reflected in Entity, so this kind of modeling is unreasonable, but also contrary to the original intention of DDD.
2. Do not impose objects of non-domain concepts on Domain:
RepeatQuery Field V, for example, is only a Search query parameter and should not be a ValueObject. It is more appropriate to define a RepeatCheckParam and put it in the Infrastructure layer, which makes it easier to understand.
Analytical field
Customer-connected Leads come mostly from inquiry contacts on the website, so we treat new inquiries as new Leads. But what about the inquiry contact modification on the other side of the website, which is obviously a matter of Contact domain. If you mix it up with Leads, it will lead to confusion.
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. It's a contact. Don't put it in Leads. event = new LeadsUpdateEvent(leads); break; case DELETE://TODO: This is not Leads. It's a contact. Don't put it in Leads. event = new LeadsDeleteEvent(leads); break; case ASSIGN://TODO: This is not Leads, don't put it in 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:
Many actions can be done on multiple Domains. At this time, we need to carefully analyze and think about which Domain is the most suitable Domain, instead of wearing headphones, looking at the code, realizing business functions, leaving everything to be done and leaving toilet paper all over the place.
Domain Behavior vs. Domain Services
To distinguish domain behavior from domain services, you can refer to the following division:
- Domain Behavior: It's within the scope of my Entity. If you add it to Entity, don't make it into a Domain Service without thinking, so Entity can easily be overhead.
- Domain Services: Not an Entity can handle behavior, you can consider extracting it to the higher level of Domain Service.
Therefore, for the action of adding new Leads, intuition should belong to Leads Entity, but after careful analysis, we will find that in addition to adding new Leads, we also need to create customers and contacts. If there is a need for allocation, the opportunity should also be allocated to the private sea of the salesman.
With so much logic and so much Entity interaction, it would be inappropriate to put Leads Entity again, so when refactoring, I abstracted Leads Process Service as a Domain Service, which would be more clear in expression and more convenient for colleagues to expand.
@Override public void processLeads(IcbuLeadsE icbuLeadsE) { IcbuLeadsE existingLeads = icbuLeadsE.checkRepeat(); //1. New clues if(existingLeads == null){ processBrandNewLeads(icbuLeadsE); } // 2 Clues already exist, but no customers have been created. else if(existingLeads.isCustomerNotCreated()){ processRepeatedLeadsWithNoCustomer(existingLeads, icbuLeadsE); } //3 Clues already exist and customers have been created to try to retrieve private seas. else{ processRepeatedLeadsAndCustomer(existingLeads, icbuLeadsE); } }
Unified Language
The language in PRD, the language we usually communicate with, the language in the model and the language in our code should be consistent. If not, or missing, it will greatly increase the cost of our code understanding and make code maintenance difficult.
For example, customer judgment and contact judgment are very important domain concepts, but they are not embodied in our domain model and should be highlighted:
/** * Customer weighting * * @return If there are duplicate customers, return their customerId, otherwise return null */ public String checkRepeat() { RepeatCheckParam repeatCheckParam = RepeatCheckParam.ofCompanyName(companyName); ... } /** * Contacts are judged by email. * @return If there are duplicate contacts, return their contactId or null */ public String checkRepeat(){ if(email == null || email.size() == 0){ return null; } //Check only the first email, because there is only one email for a contact in the icbu line of business RepeatCheckParam repeatCheckParam = RepeatCheckParam.ofContactEmail(email.iterator().next()); ... }
At present, there is checkConflict for weighting in the system, which requires the team to reach an agreement. If everyone agrees to use checkRepeat for weighting, then check Repeat will be used uniformly in the future.
This place should not be confused about the accuracy of translation, just like the concept of the high seas, we use PublicSea, which is a very obvious chinglish, but it is easy to read and understand. I think it is better for Chinese programmers than Shared Territory.
Business Semantic Explicitness
Or that sentence, the code is written for people to read, the machine can execute the code anyone can write, write people can understand the code is NB.
Here are two refactoring cases to illustrate what business semantics is explicit. Let's see the difference for ourselves.
1. The logic of judging whether Leads has created customers is to see if Leads has CustomerId.
Before reconstruction:
if (StringUtil.isBlank(existLeads.getCustomerId())
After reconstruction:
if(existingLeads.isCustomerNotCreated())
Tips: It's just a line of code, but it's a difference in programming perception.
2. Judging whether Customer is in his private sea
Before reconstruction:
public boolean checkPrivate(CustomerE customer) { IcbuCustomerE icbuCustomer = (IcbuCustomerE)customer; return !PvgContext.getCrmUserId().equals(NIL_VALUE) && icbuCustomer.getCustomerGroup() != CustomerGroup.AliCrmCustomerGroup.CANCEL_GROUP; }
After reconstruction:
/** * Judging whether it can be picked up in the private sea * @return */ public boolean isAvailableForPrivateSea(){ //If it is not operated by salesmen, it can not be found in the private sea. if(StringUtil.isBlank(PvgContext.getCrmUserId())){ return false; } //If it is a "delete group" customer, do not pick up the private sea, put it on the high sea if(this.getCustomerGroup() == CustomerGroup.AliCrmCustomerGroup.CANCEL_GROUP){ return false; } return true; }
performance optimization
Class B services are relatively less stressful, and performance is often not a bottleneck, but it does not mean that you can squander.
For example, in the Leads update operation, only one field needs to be updated, but full field updates (dozens of fields) are used, which obviously wastes resources, so a single field update SQL is added.
//Use this before refactoring <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> ... This is omitted. N Multi-field <if test="bizCode != null"> , biz_code = #{bizCode} </if> where id = #{id} and tenant_id = #{tenantId} and IS_DELETED = 'n' </update> //Use this after refactoring <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: Performance optimization needs details. Don't mention performance, just go to ThreadPool.
Test code
Look at your test code, most of them are written in disorder, not structured.
In fact, the test code can be easily structured in three steps:
- Prepare: Prepare data, including preparing request parameters and data cleansing
- Execute: Execute the code to be tested
- Check: Check execution results
Here are the test cases I wrote to create Leads, but the customer did not create them:
@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"); }
The test code allows duplication, because it allows better isolation without changing one data and affecting other test cases.
Tips: Pandora Boot starts very slowly. If it's not full mock, I recommend using my TestContainer, which can improve a lot.
Author: ali-frank
This article is the original content of Yunqi Community, which can not be reproduced without permission.