|
锁定老帖子 主题：实践中的重构01-05
精华帖 (0) :: 良好帖 (5) :: 新手帖 (0) :: 隐藏帖 (0)
|
|
|---|---|
| 作者 | 正文 |
|
发表时间：2010-11-15
piao_bo_yi 写道 然后再重构一下:
private static String getAbbreviatedEmail(String email, int prefix, int suffix) {
if(isInvalidEmail(email))
return email;
return preAbb(email,prefix) + "@" + lastAbb(email,suffix);
}
把concat换成+号（别用效率批判我，你就3个对象），把"@"从lastEmail中提取出来，因为感觉上更像是独立的。虽然你上面写注释了，但感觉不符合逻辑的地方就算有注释，也不是直观明了。 用+号我是完全赞同的。 但是如果再切割的化我是有保留的。 如果有多个地方用,则抽出去，但是整个工程只有这个地方使用，2个短的简单的相邻的重复还是可以接受的。 |
| 返回顶楼 | |
|
发表时间：2010-11-15
piao_bo_yi 写道 第三段代码的例子明显是原来就是错误的，不能算重构。而且如果你是写客户代码，你怎么能要求服务代码更改实现？？你也只能写一些判断处理了。
第四段代码为什么不直接写 return ( isNeedProxy == Boolean.TRUE); 这个我觉得你没有理解这段逻辑。 这段code本身是有问题的。 @Test
public void testBoolean() {
Boolean b = new Boolean("true");
Assert.assertFalse(b == Boolean.TRUE);
Assert.assertTrue(b);
}
这个是可以通过测试的。这个是重构发现问题的一个例子。 |
| 返回顶楼 | |
|
发表时间：2010-11-15
luckaway 写道 引用 我觉得这样大可不必。调用批处理接口，应该是符合common sense的。 即可以返回一个null,可以返回一个empty list,其他情况都是返回一个大小和传入参数个数相等且顺序一致的列表。 返回一个null不好，应该返回EmptyList，Null Object模式。如果该方法被很多地方调用，那每个地方岂不是都要判断返回的是否null。如果传入的参数ids是null，要么直接抛出IllegalArgumentException，要么直接返回EmptyList。 如果该方法在Dao上，可以不做参数检查，因为dao都是给内部的Service调用的，可以保证不传null。 大小是否一致，还是得看实际情况吧，通常情况下不会所有的id都能有对应的数据。 顺序没必要一致，万一顺序有错误，很难排查。如果真的需要，可以返回List<Map>,Map的key是id，value是id对应的数据 传null和EmptyList是各有利弊的，不能一概而论。 传EmptyList也不算是NullObject吧，算得话也是很弱的模式的应用。 没有上面是必要的，只是合适和不合适。作为客户段，传一个List，返回一个List处理起来比较自然。 当然，用Map也是一个方式。 排错的问题，我一向认为，你调用的方法，如果你不信任就不要调用好了。 |
| 返回顶楼 | |
|
发表时间：2010-11-15
最后修改：2010-11-15
tuti 写道 zhang_xzhi_xjtu 写道 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(latEmail, suffix);
}
return preEmail.concat(latEmail);
}
对这段代码提点建议： （1）方法名还是有些含混 private static String getAbbreviatedEmail(String email, int prefix, int suffix) 看不出是处理EmailAddress还是Email本身的内容。几个参数的名字也很不好理解。 Email在系统里面是特指emailAddress的，这个是不说自明的。 参数的名字是有些问题，但是考虑到是个private方法，这样比较短一点。 （2）DEFAULT_TOTAL 是什么意思，从名字上看不出任何业务含义，看逻辑好像是个Email Address最小合法长度的检查，但却用<=，而不是用<. 这个常量命名有些问题。 这个地方本身就应该是<=。 （3）方法体中，注释太多，说明变量命名的可读性要继续提高。 这段不加注释，相信不会影响阅读，公司的编码规范，有操作都要有注释的，其实我很想把方法中的注释都杀掉。 （4）prefix 和suffix 为什么是参数传入，而不是常量定义。DEFAULT_PREFIX_LENGTH， DEFAULT_SUFFIX_LENGTH的含义还不太明确。 prefix 和 DEFAULT_PREFIX_LENGTH 2个数是什么关系？ 是否会出现 prefix< DEFAULT_PREFIX_LENGTH的情况。 prefix< DEFAULT_PREFIX_LENGTH是一定成立的。prefix是对email缩写的时候，明文保留的长度。这里的上下文太多，没有介绍清楚，我的错。 |
| 返回顶楼 | |
|
发表时间：2010-11-16
最后修改：2010-11-16
最近看代码的时候，发现了一个奇怪的现象。关于调用批处理接口的问题。
如调用一个查询用户信息的接口为 UserInfo getUserInfo(String id) 则对应的批处理接口为 List<UserInfo> getUserInfoByIds(List<String> ids) --------------------------------------- 我觉得这个批处理接口应该这么定义： List<UserInfo> getUserInfos(List<UserInfo> searchCondition) |
| 返回顶楼 | |
|
发表时间：2010-11-16
最后修改：2010-11-16
kazy 写道 最近看代码的时候，发现了一个奇怪的现象。关于调用批处理接口的问题。
如调用一个查询用户信息的接口为 UserInfo getUserInfo(String id) 则对应的批处理接口为 List<UserInfo> getUserInfoByIds(List<String> ids) --------------------------------------- 我觉得这个批处理接口应该这么定义： List<UserInfo> getUserInfos(List<UserInfo> searchCondition) 恩，这个也是一个方式。 但是就实现而言，如果定义一个searchCondition，dba会抓狂的。 你想想我用很多不同的searchCondition去抓数据，db会怎样？ 当然，在某些场景下，这样的定义是挺好的。 |
| 返回顶楼 | |
