Code CR Template#
Basic Section#
-
Coding style follows Alibaba Java Development Manual.
-
For large and complex process programs, flowcharts and sequence diagrams must be provided.
- The CR colleague should briefly describe the project size and background at the beginning of the CR. For complex processes, the CR should check if the implementation logic matches the technical solution to reduce the gap between implementation and technical solution.
-
Is the business logic properly divided? Is there any coupling?
- Business logic involving domain logic should be split if it is obvious or coupled.
-
Is the code logic clear?
- The CR colleague needs to explain the main logic clearly.
-
Reasonable and effective comments.
- For complex code or logic that the CR has questions about, reasonable and effective comments are required.
-
Are there any redundant code?
- It is recommended to delete unnecessary or unreferenced code.
-
Are there any potential NPE (NullPointerException)?
-
NPE is a common bug in daily work.
Context fields should be checked for null, getXX() operations, and unboxing of primitive types. Defensive judgment or Optional is needed.
-
-
Logging printing specification.
-
It is mandatory to use placeholders for logging printing because string concatenation uses the append() method of StringBuilder, which has some performance loss.
logger.info("are u ok : oid 1", oid);
logger.info("are u ok : oid 1", oid);
-
-
Are there any Maven dependency conflicts?
- Many runtime bugs are often caused by dependency conflicts. The CR colleague should confirm if there are any changes in the POM.
-
Resource release.
- Whether it is network IO or file IO, the logic of resource release needs to be checked by the CR.
-
Unified error codes.
- Prohibit using unified error codes such as -1, 500, etc. It is best to define enums for easy problem tracing.
-
Exception handling.
- Places where exceptions occur need to confirm whether the following operations are needed: direct return, throwing exceptions, retry processing, recovery processing, circuit breaker processing, degradation processing, business closure.
Intermediate Section#
- Is the database index design reasonable and effective?
- The CR colleague needs to explain the reasons and effects of designing the index clearly.
- Can the code use design patterns or is the use of design patterns excessive?
- If there are too many if/else or scenario dispatch class logic, it is recommended to use the strategy pattern.
- It is not recommended to use complex inheritance abstraction logic for small requirements. Good design is the reasonable modeling of the model.
- It is not recommended to introduce complex frameworks to solve simple problems. Some colleagues are enthusiastic about using one framework to handle all architectures (such as DDD Cola). After all, the historical complexity of the code and the framework cannot be evaluated. Good business framework scaffolding tends to solve problems with this mindset rather than blindly copying.
- Is the middleware usage best practice?
- For example, whether MQ consumption is correctly acknowledged, whether it needs to be retried, or whether it needs to be discarded.
- For example, whether Redis has a large key design and needs to be designed reasonably.
- For example, whether the timeout setting for ES queries is reasonable.
- Thread pool usage, whether the parameters are correct, and whether there is business isolation.
- Whether to use the company's framework or the built-in parameterized thread pool construction method of JDK.
- Whether the number of threads and queue size are configured reasonably.
- Whether to configure an isolated thread pool for multiple businesses or callers.
- If locks are used, whether the lock scope and granularity are appropriate.
- The lock range of distributed locks needs to be checked for any impact on others.
- Transaction processing: whether transactions are needed and whether they are effective.
- Whether persistence operations have not added transactions.
- Whether internal calls within a class cause transactions to be ineffective.
Advanced Section#
Performance Optimization#
- Is caching added where necessary?
- For example, if the performance of the external interface that the code depends on is poor, add appropriate caching to solve query problems.
- Optimistic locking instead of pessimistic locking.
- Use multithreading to accelerate multiple interface aggregations.
Consistency#
Idempotence#
- Idempotence based on status.
- Based on status, it means that calling this interface will cause a change in status. If the status has already changed, just return the previous result.
- Idempotence based on a certain key.
- Usually, a separate field is used to bind or a log table is used to record. If it exists, just return directly.
Retry#
- Retry when interface calls fail.
-
It is not recommended to retry for external interfaces with write operations. Let the external system initiate the request again if retry is needed.
-
Check if the provided interface supports idempotent return.
-