By Wang Yaoxing (Chenglu)
Develop an Idea plug-in to realize the customized format check of the YAML file.
!! is accurate
After the code function was launched, many problems were found. The code was refactored with the help of the supervisor. The analysis and summary of the code before and after the refactoring was given. The following article will compare the code before and after refactoring from the perspectives of structural design, code readability, and robustness.
Before:
After:
Comparison:
After: Add celtVisitMapping layer code in abstract classes to act as a unified proxy for multiple code check modules. After capturing errors, you can do other unified processing (logs, identification parameters, etc.) to facilitate expansion.
A good name can output more information. It will tell you why it exists, what it does, and how it should be used.
| Feature | Time | Class Name |
| Check whether the YAML file can be deserialized into an object in the project. | Before | YamlBaseInspection |
| After | CeltClassInspection |
Comparison:
The naming of a class should be explicit, which the naming YamlBaseInspection of before cannot realize. Useful information cannot be obtained through the class name. For the CeltClassInspection naming format, you can determine that it belongs to the YAML format check based on the understanding of the plug-in function.
| Feature | Time | Function Name |
| Compare whether value can be deserialized into PsiClass. | Before | compareNameAndValue |
| After | compareKeyAndValue |
Comparison:
Before:
After:
The unit before and after the function name is the same. Key and Value are two concepts of the map in YAML. The function features can be obtained from the name: check whether the Key and Value are accurate.
//before
ASTNode node = mapping.getNode().findChildByType(YAMLTokenTypes.TAG);
String className = node.getText().substring(2);
//after
ASTNode node = mapping.getNode().findChildByType(YAMLTokenTypes.TAG);
String tagClassName = node.getText().substring(2);Comparison:
There can be two sources of String className:
After:
The variable name tagClass can be used to quickly and accurately obtain the variable name that belongs to the first of the preceding sources, which can reduce the code complexity. Variable names can offer more useful information.
Notes that conform to the JavaDoc specifications.
//before
private boolean checkSimpleValue(PsiClass psiClass, PsiElement value)
/**
* Check the value of the enumeration class.
* @return
*/
boolean checkEnum(PsiClass psiClass,String text)
//after
/**
* @param psiClass
* @param value
* @return true is normal; false is abnormal.
*/
private boolean checkSimpleValue(PsiClass psiClass, PsiElement value, ProblemsHolder holder)Before:
// simple type, check the keyName and value formats.
if (PsiClassUtil.isSimpleType(psiClass)) {
// Generic (T), Object, Whitelist: Do not check.
} else if (PsiClassUtil.isGenericType(psiClass)) {
// complex type
} else {
}After:
// The simpleValue is null or "null".
if (YamlUtil.isNull(value)) {
}
if (PsiClassUtil.isSimpleType(psiClass)) {
// simple type, check the keyName and value formats.
checkSimpleValue(psiClass, value, holder);
} else if (PsiClassUtil.isGenericType(psiClass)) {
// Generic (T), Object, Whitelist: Do not check.
} else {
checkComplexValue(psiClass, value, holder);
}Inline notes should be within the explained code block.
Before:
public void compareNameAndValue(PsiClass psiClass, YAMLValue value) {
// simple type, check the keyName and value formats.
if (PsiClassUtil.isSimpleType(psiClass)) {
// Generic (T), Object, Whitelist: Do not check.
} else if (PsiClassUtil.isGenericType(psiClass)) {
// complex type
} else {
Map<String, PsiType> map = new HashMap<>();
Map<YAMLKeyValue, PsiType> keyValuePsiTypeMap = new HashMap<>();
// init Map<KeyValue,PsiType> Register the error of keyName Error.
PsiField[] allFields = psiClass.getAllFields();
YAMLMapping mapping = (YAMLMapping) value;
Collection<YAMLKeyValue> keyValues = mapping.getKeyValues();
for (PsiField field : allFields) {
map.put(field.getName(), field.getType());
}
for (YAMLKeyValue keyValue : keyValues) {
if (map.containsKey(keyValue.getName())) {
keyValuePsiTypeMap.put(keyValue, map.get(keyValue.getName()));
} else {
holder.registerProblem(keyValue.getKey(), "This property cannot be found", ProblemHighlightType.LIKE_UNKNOWN_SYMBOL);
}
}
keyValuePsiTypeMap.forEach((yamlKeyValue, psiType) -> {
// todo: array type check
if (psiType instanceof PsiArrayType || PsiClassUtil.isCollectionOrMap(PsiTypeUtil.getPsiCLass(psiType, yamlKeyValue))) {
} else {
compareNameAndValue(PsiTypeUtil.getPsiCLass(psiType, yamlKeyValue), yamlKeyValue.getValue());
}
});
}
}After:
public void compareKeyAndValue(PsiClass psiClass, YAMLValue value, ProblemsHolder holder) {
// The simpleValue is null or "null".
if (YamlUtil.isNull(value)) {
return;
}
if (PsiClassUtil.isSimpleType(psiClass)) {
// simple type, check the keyName and value formats.
checkSimpleValue(psiClass, value, holder);
} else if (PsiClassUtil.isGenericType(psiClass)) {
// Generic (T), Object, Whitelist: Do not check.
} else {
checkComplexValue(psiClass, value, holder);
}
}
boolean checkComplexValue();Comparison:
Before:
The code of CompareNameAndValue method is too long. One screen cannot view the entire method. The framework of the method cannot be concise, which means it is responsible for judging types, distributing processing, and comparing complex types and coupling functions.
After:
The comparison of complex objects is extracted as a method, which is responsible for the comparison of complex types. The original method is only responsible for distinguishing the types and calling the actual method comparison. The method architecture can be seen, and the code is easy to maintain at a later stage.
if Complex JudgmentBefore:
After:
Comparison:
Before:
Complex if nesting is used in the code. if is one of the most important factors that make the code difficult to read. if and for loops are nested in deep V nesting. The code logic is unclear, the code maintenance is high, and the expansion is complex.
After:
It reduces if nesting, reduces the cost of code understanding, and is easy to maintain and expand.
//before
holder.registerProblem(value "The type cannot be converted", ProblemHighlightType.GENERIC_ERROR);
//after
String errorMsg = String.format("cannot find field:%s in class:%s", yamlKeyValue.getName(), psiClass.getQualifiedName());
holder.registerProblem(yamlKeyValue.getKey(), errorMsg, ProblemHighlightType.LIKE_UNKNOWN_SYMBOL);Comparison:
Before:
The error prompt for format checking is casual, only indicating that the type cannot be converted. What is from? What is to? There is no explanation. A lot of useful information has not been fed back to users. The user experience will be poor, like an immature product.
After:
It indicates that a field cannot be found in the class. It clearly states which field and class is. It helps group users locate and solve errors in a timely and accurate manner.
Before:
The code needs to consider exceptions (null pointer, unexpected scenarios). The following code has a null pointer exception, deleteSqlList may be null, the 3-line call will throw NPE, and the program does not catch processing.
YAMLKeyValue deleteSqlList = mapping.getKeyValueByKey("deleteSQLList");
YAMLSequence sequence = (YAMLSequence) deleteSqlList.getValue();
List<YAMLSequenceItem> items = sequence.getItems();
for (YAMLSequenceItem item : items) {
if (!DELETE_SQL_PATTERN.matcher(item.getValue().getText()).find()) {
holder.registerProblem(item.getValue(), "sql error", ProblemHighlightType.GENERIC_ERROR);
}
}After:
@Override
public void doVisitMapping(@NotNull YAMLMapping mapping, @NotNull ProblemsHolder holder) {
ASTNode node = mapping.getNode().findChildByType(YAMLTokenTypes.TAG);
// Remove the node.
if (YamlUtil.isNull(node)) {
return;
}
if (node.getText() == null || !node.getText().startsWith("!!")) {
// throw new RuntimeException("yaml plug-in monitoring exceptions, YAMLQuotedTextImpl text is null or not!! Beginning");
holder.registerProblem(node.getPsi(), "yaml plug-in monitoring exceptions, YAMLQuotedTextImpl text is null or not!! Beginning", ProblemHighlightType.LIKE_UNKNOWN_SYMBOL);
return;
}
String tagClassName = node.getText().substring(2);
PsiClass[] psiClasses = ProjectService.findPsiClasses(tagClassName, mapping.getProject());
if (ArrayUtils.isEmpty(psiClasses)) {
String errorMsg = String.format("cannot find className = %s", tagClassName);
holder.registerProblem(node.getPsi(), errorMsg, ProblemHighlightType.LIKE_UNKNOWN_SYMBOL);
return;
}
if (psiClasses.length == 1) {
compareKeyAndValue(psiClasses[0], mapping, holder);
}
}Exceptions are considered in each step of the operation. Lines 7, 11, and 20 have null pointer exceptions processing.
Comparison:
After:
The code handles exceptions more comprehensively, such as invalid tagString format, null pointer, and out-of-bounds array. The code is more robust.
Before:
switch (className) {
case "java.lang.Boolean":
break;
case "java.lang.Character":
break;
case "java.math.BigDecimal":
break;
case "java.util.Date":
break;
default:
}After:
switch (className) {
case "java.lang.Boolean":
break;
case "java.lang.Character":
break;
case "java.math.BigDecimal":
break;
case "java.util.Date":
case "java.lang.String":
return true;
default:
holder.registerProblem(value, "Unrecognized className:" + className, ProblemHighlightType.LIKE_UNKNOWN_SYMBOL);
return false;
}Comparison:
Before:
If the code has hidden logic, the string type will follow default logic and will not be processed. This makes the code more difficult to be understood. Exceptions are not handled for the default of the non-simple type.
After:
Write a specific case for string type to show hidden logic. Handle exceptions on default to make the code more robust.
Five Writing Skills Effectively Improving Unit Testing Practices
Code Smell - High Cyclomatic Complexity with Multi-Layer Nesting
1,029 posts | 252 followers
FollowAlibaba Cloud Community - March 24, 2023
OpenAnolis - May 10, 2023
Alibaba Clouder - May 24, 2019
Alibaba Clouder - March 11, 2021
Alibaba Clouder - November 12, 2019
Alibaba Cloud Community - September 16, 2021
1,029 posts | 252 followers
Follow
Global Internet Access Solution
Migrate your Internet Data Center’s (IDC) Internet gateway to the cloud securely through Alibaba Cloud’s high-quality Internet bandwidth and premium Mainland China route.
Learn MoreMore Posts by Alibaba Cloud Community