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

下面的类是一个老系统的代码，现在放到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

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