Java SonarQube 重构此方法以降低其认知复杂度
声明：本页面是StackOverFlow热门问题的中英对照翻译，遵循CC BY-SA 4.0协议，如果您需要使用它，必须同样遵循CC BY-SA许可，注明原文地址和作者信息，同时你必须将它归于原作者（不是我）：StackOverFlow
原文地址: http://stackoverflow.com/questions/49752350/
Warning: these are provided under cc-by-sa 4.0 license. You are free to use/share it, But you must attribute it to the original authors (not me):
StackOverFlow
SonarQube Refactor this method to reduce its Cognitive Complexity
提问by smruti ranjan
I have the below utility method and I am using multiple if statements and getting cognitive complexity issue. I went through some links, but I am not able to understand how should I change my code without affecting users of this method.
我有以下实用方法，并且我使用了多个 if 语句并遇到认知复杂性问题。我浏览了一些链接，但我无法理解如何在不影响此方法用户的情况下更改我的代码。
public static boolean isWrapperValid(WrapperClass wrapper, boolean isTechnicalToken){
String key=null;
boolean isValidWrapper = false;
if (wrapper != null && wrapper.length() > 7
&& wrapper.substring(0, 6).equalsIgnoreCase("XYZ"))
{
wrapper= wrapper.substring(7, wrapper.lastIndexOf('.')+1);
}
if(wrapper != null && wrapper.equalsIgnoreCase("TFR")) {
isValidWrapper=Boolean.TRUE;
}
try {
key = wrapper.getKey();
}
catch (Exception exception) {
return isValidWrapper;
}
if(key!=null) {
Date tokenExpiryTime = key.getExpiresAt();
if(tokenExpiryTime!=null) {
return isValidWrapper;
}
String algorithm=key.getAlgorithm();
if(!DESIRED_ALGO.equals(algorithm)) {
return isValidWrapper;
}
String value6=key.getType();
if(!DESIRED_TYPE.equals(value6)) {
return isValidWrapper;
}
if(key.getValue1()!=null && key.getValue2().size()>0 && key.getValue3()!=null && key.getValue4()!=null && key.getValue5()!=null) {
isValidWrapper=Boolean.TRUE;
}
}
return isValidWrapper;
}
Please share your suggestions to refactor this code.
请分享您对重构此代码的建议。
回答by HBo
First of all, Sonar should give you more flags: reusing the wrapperparameter is usually a bad practice, NPE where invoking wrapper.getKeybecause wrappercan be null, but anyway, not the point...
首先，声纳应该给你更多的标志：重用wrapper参数通常是一种不好的做法，NPE 调用wrapper.getKey因为wrapper可以为空，但无论如何，不是重点......
Try reducing the number of ifstatements by creating local boolean variables (or possibly 1 big ifstatement if you have less than 5 or 6 tests, but often less readable). Once it's done, you should only have 1 block testing these boolean variables, and have one return statement, like the example above (not necessarily accurate!):
尝试if通过创建局部布尔变量来减少语句的数量（if如果测试少于 5 或 6 个，则可能是 1 个大语句，但通常可读性较差）。完成后，您应该只有 1 个块来测试这些布尔变量，并有一个 return 语句，如上面的示例（不一定准确！）：
boolean expired = tokenExpiryTime != null;
boolean desiredAlgo = DESIRED_ALGO.equals(key.getAlgorithm());
boolean desiredType = DESIRED_TYPE.equals(value6);
if (expired || !desiredAlgo || !desiredType) {
return isValidWrapper;
}
However, your Cognitive complexity level seems pretty low if this kind of algorithm triggers it...
但是，如果这种算法触发它，您的认知复杂性水平似乎很低......
Another big way to reduce an algorithm complexity is to turn sub-blocks of code (loops, if and try-catch) into private methods. In your example, it could be something like a checkWrapperValiditymethod, responsible for every test returning isValidWrapper
降低算法复杂性的另一个重要方法是将代码的子块（循环、if 和 try-catch）转换为私有方法。在您的示例中，它可能类似于一个checkWrapperValidity方法，负责每个测试返回isValidWrapper
回答by Joop Eggen
A bit of rewriting delivered a simplification, that still could be improved upon.
一些重写提供了一种简化，仍然可以改进。
public static boolean isWrapperValid(WrapperClass wrapper, boolean isTechnicalToken){
if (wrapper != null && wrapper.length() > 7
&& wrapper.substring(0, 6).equalsIgnoreCase("XYZ"))
{
wrapper = wrapper.substring(7, wrapper.lastIndexOf('.')+1);
}
boolean isValidWrapper = wrapper != null && wrapper.equalsIgnoreCase("TFR");
try {
String key = wrapper.getKey();
if (key != null && key.getExpiresAt() == null
&& DESIRED_ALGO.equals(key.getAlgorithm())
&& DESIRED_TYPE.equals(key.getType())
&& key.getValue1() != null && !key.getValue2().isEmpty()
&& key.getValue3() != null && key.getValue4() != null
&& key.getValue5() != null) {
isValidWrapper = true;
}
}
catch (Exception exception) {
// DO NOTHING
}
return isValidWrapper;
}
After comment: here I catch any exception for all calls.
评论后：在这里我捕获所有调用的任何异常。
回答by agabrys
I don't think that merging many ifconditions to one or simply do a code clean up, for example by changing the order of some instructions, can solve your problem.
我不认为将许多if条件合并为一个或简单地进行代码清理（例如通过更改某些指令的顺序）可以解决您的问题。
Your code does not match the single responsibility principle. You should refactor this big method to smaller parts. Due to this it will testable, easier to maintain and read. I spent some time and did this:
您的代码不符合单一职责原则。您应该将这个大方法重构为较小的部分。因此，它将可测试，更易于维护和阅读。我花了一些时间做了这个：
public static boolean isWrapperValid(WrapperClass wrapper, boolean isTechnicalToken) {
final WrapperClass unpackedWrapper = unpackWrapper(wrapper);
boolean wrapperValid = isUnpackedWrapperValid(unpackedWrapper);
Key key = null;
try {
key = unpackedWrapper.getKey();
} catch (final Exception exception) {
return wrapperValid;
}
if (key != null) {
if (doesKeyMeetsBasicConditions(key)) {
return wrapperValid;
}
if (doesKeyMeetsValueConditions(key)) {
return true;
}
}
return wrapperValid;
}
protected static WrapperClass unpackWrapper(final WrapperClass wrapper) {
if (wrapper != null && wrapper.length() > 7 && wrapper.substring(0, 6).equalsIgnoreCase("XYZ")) {
return wrapper.substring(7, wrapper.lastIndexOf('.') + 1);
}
return wrapper;
}
protected static boolean isUnpackedWrapperValid(final WrapperClass wrapper) {
return wrapper != null && wrapper.equalsIgnoreCase("TFR");
}
protected static boolean doesKeyMeetsBasicConditions(final Key key) {
Date tokenExpiryTime = key.getExpiresAt();
if (tokenExpiryTime != null) {
return true;
}
String algorithm = key.getAlgorithm();
if (!DESIRED_ALGO.equals(algorithm)) {
return true;
}
String value6 = key.getType();
if (!DESIRED_TYPE.equals(value6)) {
return true;
}
return false;
}
protected static boolean doesKeyMeetsValueConditions(final Key key) {
return key.getValue1() != null && key.getValue2().size() > 0
&& key.getValue3() != null && key.getValue4() != null
&& key.getValue5() != null;
}
I don't know the domain logic, so some of my methods have stupid names etc. As you can see, now you have a lot of smaller methods with not many branches (ifconditions) - easier to test (a static code is not nice, but you can mock it by using for example PowerMock).
我不知道领域逻辑，所以我的一些方法有愚蠢的名字等等。正如你所看到的，现在你有很多较小的方法，没有很多分支（if条件） - 更容易测试（静态代码不好） ，但您可以使用例如PowerMock来模拟它）。
