java SonarQube重构此方法以降低其认知复杂度

eqoofvh9  于 2023-06-28  发布在  Java
关注(0)|答案(4)|浏览(186)

我有下面的实用方法,我使用多个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;
}

请分享您对重构此代码的建议。

fkaflof6

fkaflof61#

我不认为将许多if条件合并为一个或简单地进行代码清理(例如,通过更改某些指令的顺序)可以解决您的问题。
您的代码与single responsibility principle不匹配。您应该将这个大方法重构为更小的部分。由于这一点,它将可测试,更容易维护和阅读。我花了一段时间,做了这件事:

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();
    return !DESIRED_TYPE.equals(value6);
}

protected static boolean doesKeyMeetsValueConditions(final Key key) {
    return key.getValue1() != null && key.getValue2().size() > 0
           && key.getValue3() != null && key.getValue4() != null
           && key.getValue5() != null;
}

我不知道域逻辑,所以我的一些方法有愚蠢的名字等等。正如您所看到的,现在您有了很多较小的方法,分支不多(if条件)-更容易测试(静态代码并不好,但您可以通过使用例如PowerMock来模拟它)。

b4qexyjb

b4qexyjb2#

一点重写提供了一个简化,这仍然可以改进。

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;
}
  • 评论后:在这里,我捕获所有调用的任何异常。*
ar7v8xwq

ar7v8xwq3#

首先,声纳应该给予你更多的旗帜:重复使用wrapper参数通常是一个不好的做法,NPE在调用wrapper.getKey时,因为wrapper可以为null,但无论如何,这不是重点...
尝试通过创建局部布尔变量来减少if语句的数量(如果测试少于5或6个,则可能使用一个大的if语句,但通常可读性较差)。一旦完成,你应该只有一个块测试这些布尔变量,并有一个返回语句,就像上面的例子(不一定准确!):

boolean expired = tokenExpiryTime != null;
boolean desiredAlgo = DESIRED_ALGO.equals(key.getAlgorithm());
boolean desiredType = DESIRED_TYPE.equals(value6);
if (expired || !desiredAlgo || !desiredType) {
    return isValidWrapper;
}

然而,如果这种算法触发它,你的认知复杂度水平似乎很低...
降低算法复杂度的另一个重要方法是将子代码块(循环、if和try-catch)转换为私有方法。在您的示例中,它可能类似于checkWrapperValidity方法,负责每个返回isValidWrapper的测试

jw5wzhpr

jw5wzhpr4#

请在react中的特定文件顶部添加以下行,
/* eslint-disable sonarjs/cognitive-complexity */

  • 我已经使用eslint开发了react应用程序,因此可以根据您的构建平台进行更改

相关问题