透过J2Cache的吐槽,领悟代码的设计

      经常去开源社区的，大部分应该都知道J2Cache吧，不过没听说过也没关系，这 是 开源中国社区OSChina 目前正在使用的两级缓存框架，托管在OSG@Git上，大部分由设计人红薯来维护。它的第一级缓存使用 Ehcache，第二级缓存使用 Redis 。由于大量的缓存读取会导致 L2 的网络成为整个系统的瓶颈，因此 L1 的目标是降低对 L2 的读取次数。该缓存框架主要用于集群环境中。单机也可使用，用于避免应用重启导致的 Ehcache 缓存数据丢失。

       前几天，有个大牛在开源社区上写了篇博客专门吐槽J2Cache的，大牛就是大牛啊，吐槽的确实有点水准，我看了就感觉领悟了一些代码设计上的道道，当然了像我这种境界不高的人，谈领悟代码设计有点大，不过大家认真看看确实能有收获。下面我将和大家分享下大牛的吐槽和部分网友的评论，本人修道还不高，还没有能力对槽点做个好的点评并提炼出“代码的设计”还大家分享，也不能对哪些吐槽的观点评论不好，大家见谅一下，大家可以看看，领悟领悟，百益而无一害。

摘要：

呵呵，久闻J2Cache的大名，呵呵今天有点时间看了下，介个就犹豫不决了，是踩呢还是不踩呢？踩了@红薯 会不会封我的号呀，呵呵，最后想半天，最帅的红薯心胸宽广得像大海一样，那还等啥，踩吧。 就是不知道红薯会不会给个置顶机会？不给置顶给个机械键盘也行。

 

槽点一：集成方式采用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这个实现类中，就有如下问题（当然，如果非说它不是个问题，我也没有办法）：

[java]  view plain  copy

1. private static String host;  
2. private static int port;  
3. private static int timeout;  
4. private static String password;  
5. private static int database;  

 

这里声明了一堆的静态变量

结果这些变量都仅在start方法中一个里面有用到，那又变成静态变量是什么意思？

[java]  view plain  copy

1. private static JedisPool pool;  

这里定义了JedisPool是个静态变量，也就意味着只能有一个实例，也就意味着如果你要开两个独立的Redis缓冲是不可行的了，如果你想在一个工程里连接两个Redis服务器--OMG，你还是死了这条心吧，除非你重写这个类。

[java]  view plain  copy

1. if(key instanceof Number)  
2.             return region + ":I:" + key;  
3.         else{  
4.             Class keyClass = key.getClass();  
5.             if(String.class.equals(keyClass) || StringBuffer.class.equals(keyClass) || StringBuilder.class.equals(keyClass))  
6.                 return region + ":S:" + key;  
7.         }  
8.         return region + ":O:" + key;  

 

这里不断的有字符串相“+”，而且还是连加........

[java]  view plain  copy

1. public static void main(String[] args) {  
2.     RedisCache cache = new RedisCache("User");  
3.     System.out.println(cache.getKeyName("Hello"));  
4.     System.out.println(cache.getKeyName(2));  
5.     System.out.println(cache.getKeyName((byte)2));  
6.     System.out.println(cache.getKeyName(2L));  
7. }  

 

这里还有这个，还能再任性一点么？

[java]  view plain  copy

1. public Object get(Object key) throws CacheException {  
2.     Object obj = null;  
3.     boolean broken = false;  
4.     Jedis cache = RedisCacheProvider.getResource();  
5.     try {  
6.         if (null == key)  
7.             return null;  
8.         byte[] b = cache.get(getKeyName(key).getBytes());  
9.         if(b != null)  
10.             obj = SerializationUtils.deserialize(b);  
11.     } catch (Exception e) {  
12.         log.error("Error occured when get data from L2 cache", e);  
13.         broken = true;  
14.         if(e instanceof IOException || e instanceof NullPointerException)  
15.             evict(key);  
16.     } finally {  
17.         RedisCacheProvider.returnResource(cache, broken);  
18.     }  
19.     return obj;  
20. }  
21.    
22. @Override  
23. public void put(Object key, Object value) throws CacheException {  
24.     if (value == null)  
25.         evict(key);  
26.     else {  
27.         boolean broken = false;  
28.         Jedis cache = RedisCacheProvider.getResource();  
29.         try {  
30.             cache.set(getKeyName(key).getBytes(), SerializationUtils.serialize(value));  
31.         } catch (Exception e) {  
32.             broken = true;  
33.             throw new CacheException(e);  
34.         } finally {  
35.             RedisCacheProvider.returnResource(cache, broken);  
36.         }  
37.     }  
38. }  

这里是啥意思？当你拿到一个Cache对象，你每次添加或返回一个缓冲的值，用的居然不一定是一个Redis实例，好吧，我知道Redis的连接池还是比较快的？还是比较快就不花时间了么？

[java]  view plain  copy

1. if (value == null)  
2.             evict(key);  

 

Ok，这里表示，如果设置的值是null，就把这个值干掉，

[java]  view plain  copy

1. public void evict(List keys) throws CacheException {  
2.         if(keys == null || keys.size() == 0)  
3.             return ;  
4.         boolean broken = false;  
5.         Jedis cache = RedisCacheProvider.getResource();  
6.         try {  
7.             String[] okeys = new String[keys.size()];  
8.             for(int i=0;i<okeys.length;i++){  
9.                 okeys[i] = getKeyName(keys.get(i));  
10.             }  
11.             cache.del(okeys);  
12.         } catch (Exception e) {  
13.             broken = true;  
14.             throw new CacheException(e);  
15.         } finally {  
16.             RedisCacheProvider.returnResource(cache, broken);  
17.         }  
18.     }  

 

但是关键是这个方法里面又去获得了一个新的Redis对象。

[java]  view plain  copy

1. public EhCache buildCache(String name, boolean autoCreate, CacheExpiredListener listener) throws CacheException {  
2.      EhCache ehcache = _CacheManager.get(name);  
3.      if(ehcache == null && autoCreate){  
4.          try {  
5.              synchronized(_CacheManager){  
6.                  ehcache = _CacheManager.get(name);  
7.                  if(ehcache == null){  
8.                      net.sf.ehcache.Cache cache = manager.getCache(name);  
9.                      if (cache == null) {  
10.                          log.warn("Could not find configuration [" + name + "]; using defaults.");  
11.                          manager.addCache(name);  
12.                          cache = manager.getCache(name);  
13.                          log.debug("started EHCache region: " + name);                  
14.                      }  
15.                      ehcache = new EhCache(cache, listener);  
16.                      _CacheManager.put(name, ehcache);  
17.                  }  
18.              }  
19.          }  
20.          catch (net.sf.ehcache.CacheException e) {  
21.              throw new CacheException(e);  
22.          }  
23.      }  
24.      return ehcache;  
25.  }  

 

这个方法写得实在是醉了，呵呵，太难看了。

这里关键在于：如果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%以下的
• 悠然判定认为需要请客的

 

部分评论：

1、感谢博主犀利的点评！让我等码神坐立不安。我想说的是：人不在高，够骚就行；码不在多，有神则灵。红薯，加油！其实你可以更好的！
2、炸了，OSC社区需要多一些tiny老大这样code review
3、IDEA能给不少建议
4、看得我都不敢写代码了。。
5、这个确实有一些问题，估计是比较随意吧
6、槽点 必然存在！ 但不必过于在意。 注意 是过于。毕竟优秀的开源框架 是在一个系统的设计上和更多好而实用的新特性中不断迭代开发完成，忽然一天或者慢慢的被大多数开发者所认同，接受。 支持@红薯。ps：非粉
7、... 好多地方是我写的！maven是我搞的！
8、我来背锅！
9、@红薯 发飙了
10、..................

 

       其实，槽点也是必须存在的，一个开源的，花费自己经历和金钱去维护，去和大家分享，完全是基于一种爱好，一种奉献，毕竟各个方面还是是有限的。但是吐槽归吐槽，重点在于建议，在于Idea，通过吐槽，大家分享一下各自的建议并且能学到一些东西，我觉得通过这个吐槽我绝学到了一些代码上的设计和规范性，所以特意拿出时间来以文章的形式分享出来，希望大家也能发觉到里边的好东西，也只有通过吐槽，聆听大家的吐槽，才可以把相关的东西设计的更好，做的更完美，在此祝愿J2Cache越来越强大，越来越完善！

 

升级版：扒掉红薯的内裤-深入剖析J2Cache，对接口、类的设计有很多借鉴意义，感兴趣的同学可以看看。