Recently, a popup event of a treasure led to the deletion of its APP by a large number of users, with extremely bad influence. I wonder if their internal code review had been a little more rigorous and a little less formal, they could have nip it in the bud. Based on this, our department specially set up a code review group composed of team leaders and core members to optimize the previous code review mode and strengthen the control. I have found some common problems in past code reviews, summarized below.
1. Lack of change description
Usually there’s only one reason I don’t write comments. These lines of code are too simple, and the meaning can be translated from the code, but some people write code that doesn’t have comments throughout, and I get confused. Am I being too lazy?
To be honest, comments and change specifications are a pair of villains that we often ignore and don’t even notice in open source frameworks. It’s not hard to see the framework’s code comments and change notes abound, including usage examples, version numbers to start supporting, etc. It’s also not hard to see our code base submitting changes with “Update” all over the screen, expecting to turn to the next page, only to be slapped in the face. Maybe it’s a matter of norms, or maybe it’s because we haven’t started greeting our predecessors yet.
Abuse of exception catching
I’ve seen some strange operations in projects where each controller method uses a global try-catch exception catch and then handles the drop in availability in its catch method. It’s easy to think in a template-like way, which you might not think is wrong, but it’s not elegant. We could have used the global section for uniform exception handling and buried the rate drop according to the returned error code. If the controller method does not return a uniform canonical result object, stop and start refactoring! In addition, the method before optimization is easy to let us relax on the intermediate results, after all, there are such a large and complete exception capture, is it possible to omit all non-null judgments or illegal judgments? !
The above problems do not affect the operation of the code at all, but when the scenario switches to transaction operations, you have to be careful because swallowing exceptions can cause transaction rollback to not be executed. So, if the caller cares about exceptions, throw them up!
3. Over-trusting third parties
Excessive trust in the meaning of third parties, including parameters passed from the front end without checking, results returned by calling upstream interfaces without checking, and calls to write data interfaces provided by other teams without checking parameters. The last example may lead to SQL vulnerability attacks, although the main responsibility is not itself. However, IN real development, I almost experienced a library deletion run because I trusted the third party too much. I copied a few lines of code from the class and performed the update operation when the parameter was not empty, but when the parameter was not passed by the front end, the parameter type was unwrapped class, which is non-empty by default, thus overwriting the database record. This error is also novice processing null value judgment easy to make.
4. The scope of variables is too large
We inadvertently start playing around with the scope of variables, such as creating and assigning variables in two different ways, which obviously adds uncertainty to the system. In addition, too many variables can drown out the trunk logic. If they are related or collaborate, they should be managed in a common way.
5. The treatment process lacks stage results
No programmer likes to read a lot of judgment statements, and quick return of early exception judgment is a basic optimization method, such as returning directly when validation is not valid, or removing control markers to reduce loop nesting. A higher level of semantics means that the process should have periodic results, so that the code is more layered and the overall flow is clearer.
6. Diary printing problems
I believe that most companies have adopted a microservices architecture, and one of the difficulties of this architecture is the rapid detection of problems, especially when it involves cross-team. And the encapsulation diary information is our handy tool, record parameters, intermediate results, returned results, abnormal information, with these information, find upstream feedback problems are more confident, without the need to modify the code to add diary after the online. So, log as many exceptions as possible, and add as many info-level journals as possible. However, journaling is bypass logic, and the loss of nullpointer exceptions or memory overruns is outweighed by journaling, so avoid serializing empty objects and using unbounded asynchronous queues when journaling.
To conclude, I have shared with you six common mistakes, including lack of change specification, abuse of exception catching, over-reliance on third parties, over-scoping of variables, lack of periodic results from processing, and journal printing issues. Hope to help you, welcome to share.
