实践中的重构01-05

目录

实践中的重构01_小方法的细调
实践中的重构02_代码的视觉效果
实践中的重构03_批处理方法默认约定
实践中的重构04_了解每一行代码 装箱的布尔值
实践中的重构05_简洁的代码


[b]实践中的重构01_小方法的细调[/b]

重构的概念已经为广大的程序员所熟悉。但是还是有很多细节可以注意。

    public static String getHiddenEmail(String email, int prefix, int suffix) {
        // 仅对包含@的email账户进行处理
        if (StringUtil.isBlank(email) || !StringUtil.contains(email, "@")) {
            return email;
        }

        int length = email.length();

        if (length < DEFAULT_TOTAL) {
            return email;
        }
        // @所在位置
        int splitPos = StringUtil.lastIndexOf(email, '@');
        // @前的字符段
        String preEmail = StringUtil.substring(email, 0, splitPos);
        if (StringUtil.isBlank(preEmail)) {
            return email;
        }
        // @后的字符段，包含@
        String latEmail = StringUtil.substring(email, splitPos, length);
        if (StringUtil.isBlank(latEmail)) {
            return email;
        }
        if (preEmail.length() > 17)
            preEmail = StringUtil.abbreviate(preEmail, prefix);

        if (latEmail.length() > 13)
            latEmail = StringUtil.abbreviate(latEmail, suffix);

        return preEmail.concat(latEmail);
    }

以上的code在我看来，有以下几个缺点。
1 方法名和实际的方法体不匹配。
2 魔数
3 if (StringUtil.isBlank(latEmail)) 永远返回false，因为当程序执行到这里，latEmail至少包括一个字符@。
4 方法中的代码的内聚性不够。主要是对preEmail和latEmail的处理分开了。
5 拼写错误，应该为lastEmail而误拼为latEmail

清洗代码后如下

private static String getAbbreviatedEmail(String email, int prefix, int suffix) {

        if (StringUtil.isBlank(email)) {
            return email;
        }

        if (email.length() <= DEFAULT_TOTAL) {
            return email;
        }

        // @所在位置
        int splitPos = StringUtil.lastIndexOf(email, '@');
        if (splitPos == -1 || splitPos == 0) {
            return email;
        }

        // @前的字符段
        String preEmail = StringUtil.substring(email, 0, splitPos);
        if (preEmail.length() > DEFAULT_PREFIX_LENGTH) {
            preEmail = StringUtil.abbreviate(preEmail, prefix);
        }

        // @后的字符段，包含@
        String lastEmail = StringUtil.substring(email, splitPos, email.length());
        if (lastEmail.length() > DEFAULT_SUFFIX_LENGTH) {
            lastEmail = StringUtil.abbreviate(lastEmail, suffix);
        }

        return preEmail+lastEmail;
    }

其实我个人最喜欢的风格是简单的方法的guard condition不用大括号{}，这样代码变为

private static String getAbbreviatedEmail(String email, int prefix, int suffix) {

        if (StringUtil.isBlank(email)) 
            return email;

        if (email.length() <= DEFAULT_TOTAL) 
            return email;

        // @所在位置
        int splitPos = StringUtil.lastIndexOf(email, '@');
        if (splitPos == -1 || splitPos == 0) 
            return email;

        // @前的字符段
        String preEmail = StringUtil.substring(email, 0, splitPos);
        if (preEmail.length() > DEFAULT_PREFIX_LENGTH) {
            preEmail = StringUtil.abbreviate(preEmail, prefix);
        }

        // @后的字符段，包含@
        String lastEmail = StringUtil.substring(email, splitPos, email.length());
        if (lastEmail.length() > DEFAULT_SUFFIX_LENGTH) {
            lastEmail = StringUtil.abbreviate(lastEmail, suffix);
        }

        return preEmail+lastEmail;
    }

[b]实践中的重构02_代码的视觉效果[/b]

相信程序员都会承认读代码的时间比写代码的时间长。那么有没有写代码的时候有没有什么可以帮助其他程序员甚至自己快速读程序的手段呢。

试看一例，当然这里给的日志很简单，实际中对于重要操作，日志是要打很多东西的。

try{
//一个操作，有可能抛出异常
db.operate();
}catch(Exception e){
log.warn(e);
}

log.info("msg");

从视觉上看，不是很清晰。


try{
//一个操作，有可能抛出异常
db.operate();
log.info("msg");
}catch(Exception e){
log.warn(e);
}

ok，不用思考了，一眼瞟过去，就知道一个log是成功日志，一个是失败日志。


[b]实践中的重构03_批处理方法默认约定[/b]

最近看代码的时候，发现了一个奇怪的现象。关于调用批处理接口的问题。

如调用一个查询用户信息的接口为
UserInfo getUserInfo(String id)

则对应的批处理接口为
List<UserInfo> getUserInfoByIds(List<String> ids)

很多地方对该返回值进行了校验，即用for循环比对返回的UserInfo进行比对，担心返回的列表的长度和传入参数的长度不同，担心返回的列表的顺序和传入参数的顺序不同。

我觉得这样大可不必。调用批处理接口，应该是符合common sense的。
即可以返回一个null,可以返回一个empty list,其他情况都是返回一个大小和传入参数个数相等且顺序一致的列表。

如果有特殊情况，应该在方法的接口定义中特别声明，这样调用方的code会比较清晰好读，也符合一般人的直觉。让调用方做校验，这样的想法如果没有很强大的理由，还是不要的好，因为遵守默认约定，有可能服务方的代码会稍微复杂一点，但是考虑到多处调用方的代码的简洁和易读，这点代价完全是值得的。


[b]实践中的重构04_了解每一行代码 装箱的布尔值[/b]

最近看到代码中有这样的code.

Boolean isNeedProxy = (Boolean)threadLocalMap.get(ip);
return ( isNeedProxy ==  Boolean.TRUE ) ? true : false;

我的猜想是编程的人为了防止isNeedProxy为null，所以有了这段代码。

这里有个问题。

如果存储的值是new出来的Boolean，那么这里的逻辑就是错的。

@Test
	public void testBoolean() {
		Boolean b = new Boolean("true");
		Assert.assertFalse(b == Boolean.TRUE);
		Assert.assertTrue(b);
	}

这样写就好了

return isNeedProxy==null?false:isNeedProxy;

[b]实践中的重构05_简洁的代码[/b]

if (a|| b|| c) {
   return true;
}
return false;

可以改为

return a|| b|| c;