我有下面的实用方法,我使用多个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;
}
请分享您对重构此代码的建议。
4条答案
按热度按时间fkaflof61#
我不认为将许多
if
条件合并为一个或简单地进行代码清理(例如,通过更改某些指令的顺序)可以解决您的问题。您的代码与single responsibility principle不匹配。您应该将这个大方法重构为更小的部分。由于这一点,它将可测试,更容易维护和阅读。我花了一段时间,做了这件事:
我不知道域逻辑,所以我的一些方法有愚蠢的名字等等。正如您所看到的,现在您有了很多较小的方法,分支不多(
if
条件)-更容易测试(静态代码并不好,但您可以通过使用例如PowerMock来模拟它)。b4qexyjb2#
一点重写提供了一个简化,这仍然可以改进。
ar7v8xwq3#
首先,声纳应该给予你更多的旗帜:重复使用
wrapper
参数通常是一个不好的做法,NPE在调用wrapper.getKey
时,因为wrapper
可以为null,但无论如何,这不是重点...尝试通过创建局部布尔变量来减少
if
语句的数量(如果测试少于5或6个,则可能使用一个大的if
语句,但通常可读性较差)。一旦完成,你应该只有一个块测试这些布尔变量,并有一个返回语句,就像上面的例子(不一定准确!):然而,如果这种算法触发它,你的认知复杂度水平似乎很低...
降低算法复杂度的另一个重要方法是将子代码块(循环、if和try-catch)转换为私有方法。在您的示例中,它可能类似于
checkWrapperValidity
方法,负责每个返回isValidWrapper
的测试jw5wzhpr4#
请在react中的特定文件顶部添加以下行,
/* eslint-disable sonarjs/cognitive-complexity */