槽点一:集成方式采用ANT
工程还是传统的工程,集成还在用的ant,当然ant来做没有什么不可以,但是作为OSC这么高大上的作者拿出的作品能不能槽点高一些?已经采用了Maven了,用Maven无法独立完成的意思么?
槽点二:提交内容的不审慎
居然把Eclipse的工程文件提交到配置库,难道不知道有N多种开发工具么?不知道Eclipse工程文件中有一些东西是与本机相关的么?这样冲突如何解决呢?难不成所有作者的工程环境全是一样的?
槽点三:文件治理不够严谨
在src/main/java中赫然有xml和properties文件存在,OMG,我们的节操还可以再低一点么?测试性的代码放在了main/java中。
槽点四:架构设计比较乱
在我看来,不管是ehcache或者redis都是缓存的一个扩展,但是这些东西都被结结实实的依赖在框架的main主干中,Hibernate只是应用缓冲的一种应用场景,但是也赫然在工程结构当中,如果把J2Cache的缓冲实现扩展N种,再把J2Cache的应用推广M种,我们就欣然看到在J2Cache当中信赖了n+m种的依赖包们,我们的使用者应该怎么办呢?要么忍受利用J2Cache时带来的N多用不着的包,要么就要辛苦的去强制不依赖这些包,而这都要拜J2Cache所赐。
槽点五:程序实现也有一些问题简单列列如下
在RedisCacheProvider这个实现类中,就有如下问题(当然,如果非说它不是个问题,我也没有办法):
1
2
3
4
5
|
private
static
String host;
private
static
int
port;
private
static
int
timeout;
private
static
String password;
private
static
int
database;
|
结果这些变量都仅在start方法中一个里面有用到,那又变成静态变量是什么意思?
1
|
private
static
JedisPool pool;
|
这里定义了JedisPool是个静态变量,也就意味着只能有一个实例,也就意味着如果你要开两个独立的Redis缓冲是不可行的了,如果你想在一个工程里连接两个Redis服务器--OMG,你还是死了这条心吧,除非你重写这个类。
1
2
3
4
5
6
7
8
|
if
(key
instanceof
Number)
return
region +
":I:"
+ key;
else
{
Class keyClass = key.getClass();
if
(String.
class
.equals(keyClass) || StringBuffer.
class
.equals(keyClass) || StringBuilder.
class
.equals(keyClass))
return
region +
":S:"
+ key;
}
return
region +
":O:"
+ key;
|
1
2
3
4
5
6
7
|
public
static
void
main(String[] args) {
RedisCache cache =
new
RedisCache(
"User"
);
System.out.println(cache.getKeyName(
"Hello"
));
System.out.println(cache.getKeyName(
2
));
System.out.println(cache.getKeyName((
byte
)
2
));
System.out.println(cache.getKeyName(2L));
}
|
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
27
28
29
30
31
32
33
34
35
36
37
38
|
public
Object get(Object key)
throws
CacheException {
Object obj =
null
;
boolean
broken =
false
;
Jedis cache = RedisCacheProvider.getResource();
try
{
if
(
null
== key)
return
null
;
byte
[] b = cache.get(getKeyName(key).getBytes());
if
(b !=
null
)
obj = SerializationUtils.deserialize(b);
}
catch
(Exception e) {
log.error(
"Error occured when get data from L2 cache"
, e);
broken =
true
;
if
(e
instanceof
IOException || e
instanceof
NullPointerException)
evict(key);
}
finally
{
RedisCacheProvider.returnResource(cache, broken);
}
return
obj;
}
@Override
public
void
put(Object key, Object value)
throws
CacheException {
if
(value ==
null
)
evict(key);
else
{
boolean
broken =
false
;
Jedis cache = RedisCacheProvider.getResource();
try
{
cache.set(getKeyName(key).getBytes(), SerializationUtils.serialize(value));
}
catch
(Exception e) {
broken =
true
;
throw
new
CacheException(e);
}
finally
{
RedisCacheProvider.returnResource(cache, broken);
}
}
}
|
这里是啥意思?当你拿到一个Cache对象,你每次添加或返回一个缓冲的值,用的居然不一定是一个Redis实例,好吧,我知道Redis的连接池还是比较快的?还是比较快就不花时间了么?
1
2
|
if
(value ==
null
)
evict(key);
|
Ok,这里表示,如果设置的值是null,就把这个值干掉,
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
|
public
void
evict(List keys)
throws
CacheException {
if
(keys ==
null
|| keys.size() ==
0
)
return
;
boolean
broken =
false
;
Jedis cache = RedisCacheProvider.getResource();
try
{
String[] okeys =
new
String[keys.size()];
for
(
int
i=
0
;i<okeys.length;i++){
okeys[i] = getKeyName(keys.get(i));
}
cache.del(okeys);
}
catch
(Exception e) {
broken =
true
;
throw
new
CacheException(e);
}
finally
{
RedisCacheProvider.returnResource(cache, broken);
}
}
|
但是关键是这个方法里面又去获得了一个新的Redis对象。
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
|
public
EhCache buildCache(String name,
boolean
autoCreate, CacheExpiredListener listener)
throws
CacheException {
EhCache ehcache = _CacheManager.get(name);
if
(ehcache ==
null
&& autoCreate){
try
{
synchronized
(_CacheManager){
ehcache = _CacheManager.get(name);
if
(ehcache ==
null
){
net.sf.ehcache.Cache cache = manager.getCache(name);
if
(cache ==
null
) {
log.warn(
"Could not find configuration ["
+ name +
"]; using defaults."
);
manager.addCache(name);
cache = manager.getCache(name);
log.debug(
"started EHCache region: "
+ name);
}
ehcache =
new
EhCache(cache, listener);
_CacheManager.put(name, ehcache);
}
}
}
catch
(net.sf.ehcache.CacheException e) {
throw
new
CacheException(e);
}
}
return
ehcache;
}
|
这里关键在于:如果autoCreate是个false,你永远拿到的是null。那么问题来了,如果autoCreate只能是true,这个参数还有意义么?
再一个问题,如果返回一个null,让使用的同学们如何用?写成下面这个样子么??
if(xxx!=null){
doSomeThing....
}
忽然在net.oschina.j2cache中又发现一个类CacheTester.java
总结一下:
J2Cache比我想像中要小许多,简单易用,当然再注重一下细节就更好了。
附:Tiny成员会出现请客的情况:
- System.out.print用在不应该出现的地方
- 不适当的使用main方法。
- 方法函数行数太长
- 圈复杂度太大--超过10
- 异常处理处理不好:应该有没有,不应该有有了,抓到异常没有说明也没有任何处理
- 造成Maven循环依赖了
- 已经测试完成的代码,出现导致整个系统崩溃的问题了
- 提交了编译通不过的代码到配置库的
- 提交了不应该提交的内容到配置库的
- 已修正Issue但是不在git上关闭的。
- Sonar规则检查通过率85%以下的
- 悠然判定认为需要请客的
Tiny的开源工路径在:http://git.oschina.net/tinyframework/tiny/ 欢迎前来点评吐槽
文档路径:http://www.tinygroup.org/ 然后传送门-》Document
写过一篇与代码质量相关的:悠然乱弹:从一段代码讲开去
写过一篇文章:从几个方法的重构讲开去--引言 ,这篇文章深深的伤透了@黄勇 大仙的心,连我去上海噌饭的时候,宁愿跑到美国,也不愿见我。
以前写过一篇文章,严格来说是QQ沟通记录,200行高质量的代码,也不是那么容易完成的,希望看看高质量代码怎么写的同学可以去看看。