如何重构一个圈复杂度超30的类

Published on:
Tags: java refactor

下面的类是一个老系统的代码，现在放到 sonar 上面进行扫描，扫出来的结果发现复杂度超过了 30。

代码复杂度是指代码中的分支数量，比如有一个 if 分支，代码复杂度就加 1，如果 if 中有“||”或者“&&”那么代码复杂度就加 2，for 和 while 同理。一般复杂度超过 10 的类就算是比较复杂的了，而这个类的复杂度竟然达到了 30，代码的糟糕程度可见一斑，现在我们就来重构一下这个类的代码。

原始文件在这里。

重构开始吧！

多处 String 类型非空判断

1
2
3
4
5
6
if (StringUtil.isEmpty(username))
    throw new ICRClientException("username can not be null");
if (StringUtil.isEmpty(password))
    throw new ICRClientException("password can not be null");
if (udto == null)
throw new ICRClientException("ICRUploadDTO can not be null");

重构之后：

1
2
3
4
5
6
7
8
9
10
11
//将原来的地方替换为
checkStringParamEmpty(username, "username");
checkStringParamEmpty(password, "password");
checkStringParamEmpty(udto.getUrlPath(), "urlPath");
...
//新增一个方法
private void checkStringParamEmpty(String value, String name) throws ICRClientException {
    if (StringUtil.isEmpty(value)) {
        throw new ICRClientException(name + " can not be null");
    }
}

原代码中不止这 3 个参数的校验，还有很多，越多参数的校验，我们重构后的复杂度就会越低。

代码复杂度变化：原来是 3，修改后为 1。

多 String 值判断

1
2
3
if (!udto.getPriority().equals("0") && !udto.getPriority().equals("1")
&& !udto.getPriority().equals("2") && !udto.getPriority().equals("3"))
    throw new ICRClientException("priority must be 0/1/2/3");

重构之后：

1
2
3
4
5
6
7
8
9
//将原来代码替换为
checkValueWithinList(udto.getPriority());
...
//新增一个方法：
private void checkValueWithinList(String priority) throws ICRClientException {
    if (!Arrays.asList("0", "1", "2", "3").contains(priority)) {
        throw new ICRClientException("priority must be 0/1/2/3");
    }
}

代码复杂度变化：原来是 4，修改后为 1。

对 list 的非空判断

1
2
if (list == null || list.size() == 0)
    throw new ICRClientException("list can not be null");

重构之后：

1
2
3
4
5
6
7
//将原来的代码替换为
checkValueWithinList(udto.getPriority());
...
//新增一个方法
private void checkListNoNull(List list) throws ICRClientException {
    if (list.isEmpty()) throw new ICRClientException("list can not be null");
}

代码复杂度变化：原来是 2，修改后为 1。

多个 catch 的内容相同

1
2
3
4
5
6
7
8
int code = 0;
try {
    code = httpClient.executeMethod(post);
} catch (HttpException e) {
    throw new ICRClientException(e.getMessage(), e);
} catch (IOException e) {
    throw new ICRClientException(e.getMessage(), e);
}

重构之后：

1
2
3
4
5
6
7
8
9
10
11
12
13
//将原来的地方替换为
int code = executeHttpClient(httpClient, post);
...
//新增一个方法
private int executeHttpClient(HttpClient httpClient, PostMethod post) throws ICRClientException {
    int code;
    try {
        code = httpClient.executeMethod(post);
    } catch (Exception e) {
        throw new ICRClientException(e.getMessage(), e);
    }
    return code;
}

代码复杂度变化：原来是 2，修改后为 1。

if 判断结果复杂化

1
2
3
4
5
6
7
8
9
10
11
12
13
14
if (code == 200) {
try {
    if (post.getResponseBodyAsString().equals("ok")) {
    return true;
}
} catch (IOException e) {
    throw new ICRClientException(e.getMessage(), e);
}
return false;
} else if (code == 500) {
    throw new ICRClientException(post.getResponseBodyAsString());
} else {
    throw new ICRClientException(code + ":" + post.getStatusText());
}

重构之后:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
//将原来代码替换为
return returnFinialResult(post, code);
...
//新增一个方法
private boolean returnFinialResult(PostMethod post, int code) throws ICRClientException, IOException {
if (code == 500) throw new ICRClientException(post.getResponseBodyAsString());
if (code != 200) throw new ICRClientException(code + ":" + post.getStatusText());

    try {
        return post.getResponseBodyAsString().equals("ok");
    } catch (IOException e) {
        throw new ICRClientException(e.getMessage(), e);
    }

}

代码复杂度变化：原来是 4，修改后为 3。

本地变量始终不为 null

1
2
3
4
5
6
7
8
9
10
11
12
13
14
public boolean uploadToICR(String username, String password, ICRUploadDTO udto) throws ICRClientException {
    HttpClient httpClient = null;
    PostMethod post = null;
    httpClient = new HttpClient();
    //some code here
    …
    } finally {
        if (post != null) {
            post.releaseConnection();
        }
    if (httpClient != null) {
        httpClient.getHttpConnectionManager().closeIdleConnections(0);
    }
}

重构之后：

1
2
3
4
5
6
7
8
9
10
11
public boolean uploadToICR(String username, String password, ICRUploadDTO udto) throws ICRClientException {
    HttpClient httpClient = new HttpClient();
    PostMethod post = null;
    //some code here
    …
    } finally {
        if (post != null) {
            post.releaseConnection();
        }
    }
}

代码复杂度变化：原来是 1，修改后为 0。

读取 IO 流的方法，为什么要自己实现？

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
private byte[] readData(InputStream ins) throws IOException {
    byte[] buf = new byte[2048];
    int count = 0;
    int len = 0;
    byte data[] = new byte[2048];
    byte[] result = null;
    try {
        while ((len = ins.read(data, 0, 2048)) != -1) {
            int newcount = count + len;
            if (newcount > buf.length) {
            byte newbuf[] = new byte[Math
            .max(buf.length << 1, newcount)];
            System.arraycopy(buf, 0, newbuf, 0, count);
            buf = newbuf;
        }
        System.arraycopy(data, 0, buf, count, len);
        count = newcount;
        }
        result = new byte[count];
        System.arraycopy(buf, 0, result, 0, count);
    } finally {
        ins.close();
    }
    return result;

}

在原代码里面自己实现了一个对读取 IO 流字节的方法，这个可以使用 apache-io 或者 guava 的 API 代替：

1
2
3
4
//使用 apache io API 的实现：
byte[] bytes = IOUtils.toByteArray(inputStream);
//使用 guava API 的实现：
byte[] bytes1 = ByteStreams.toByteArray(inputStream);

代码复杂度变化：原来是很多，修改后为 0。

最终重构后的版本见这里，最后的代码复杂度从原来的 30 降到了 3。

代码写的比较仓促，没有写单元测试，其实最好的做法是在重构之前先写好单元测试，然后再慢慢修改原来的代码，每修改一处地方跑一遍单元测试，这样可以保证你的重构没有破坏原来的代码逻辑。

赞赏

如果文章对您有所帮助，可以捐赠我喝杯咖啡😌，捐赠方式：

BTC 地址：3LYgSyf7ddMALwGWPQr3PY4wzjsTDdg1oV
ETH 地址：0x0C9b27c89A61aadb0aEC24CA8949910Cbf77Aa73
qrcode

文章已同步更新公众号，欢迎关注

qrcode

Comments