再次吐槽红薯的J2Cache

2019独角兽企业重金招聘Python工程师标准>>>

序言

以前写过两篇吐槽@红薯 的文章，吐槽一下J2Cache和扒掉红薯的内裤-深入剖析J2Cache，应该说受到了广大Oscer们的一致认同，作为一个站长不好好的搞网站非要搞程序，说什么要做站长里面代码写得最好的，写代码里面网站办的最好的。好吧，这些我都忍了，但是，居然赤果果的点名叫嚣：

作为一个程序员，作为一个连续扒过两次红薯内裤的喷子，为了维护程序员的尊严，为了广大OSCER们不要掉到红薯的坑里，为了世界和平，必须狠狠的砍他。

正文

槽点一：目录结构不规范

上眼首先来找test目录想看看这么测试和使用的，居然找不到，再仔细看居然有个Java类叫

CacheTester

再看resource和src居然不在main目录下，是我已经out了吗？

修改方式，完全按照Java&Maven的开发和代码组织规范老老实实的执行。

槽点二：测试方式不规范

下面就是所谓的测试代码了

/**
 * 
 */
package net.oschina.j2cache;

import java.io.BufferedReader;
import java.io.InputStreamReader;

/**
 * 缓存测试入口
 * @author Winter Lau
 */
public class CacheTester {

   public static void main(String[] args) {
      
      System.setProperty("java.net.preferIPv4Stack", "true"); //Disable IPv6 in JVM
      
      CacheChannel cache = J2Cache.getChannel();
      BufferedReader in=new BufferedReader(new InputStreamReader(System.in));

       do{
           try {
               System.out.print("> "); 
               System.out.flush();
               
               String line=in.readLine().trim();
               if(line.equalsIgnoreCase("quit") || line.equalsIgnoreCase("exit"))
                   break;

               String[] cmds = line.split(" ");
               if("get".equalsIgnoreCase(cmds[0])){
                  CacheObject obj = cache.get(cmds[1], cmds[2]);
                  System.out.printf("[%s,%s,L%d]=>%s\n", obj.getRegion(), obj.getKey(), obj.getLevel(), obj.getValue());
               }
               else
               if("set".equalsIgnoreCase(cmds[0])){
                  cache.set(cmds[1], cmds[2],cmds[3]);
                  System.out.printf("[%s,%s]<=%s\n",cmds[1], cmds[2], cmds[3]);
               }
               else
               if("evict".equalsIgnoreCase(cmds[0])){
                  cache.evict(cmds[1], cmds[2]);
                  System.out.printf("[%s,%s]=>null\n",cmds[1], cmds[2]);
               }
               else
               if("clear".equalsIgnoreCase(cmds[0])){
                  cache.clear(cmds[1]);
                  System.out.printf("Cache [%s] clear.\n" , cmds[1]);
               }
               else
               if("help".equalsIgnoreCase(cmds[0])){
                  printHelp();
               }
               else{
                  System.out.println("Unknown command.");
                  printHelp();
               }
           }
           catch(ArrayIndexOutOfBoundsException e) {
               System.out.println("Wrong arguments.");
              printHelp();
           }
           catch(Exception e) {
              e.printStackTrace();
           }
       }while(true);
       
       cache.close();
       
       System.exit(0);
   }
   
   private static void printHelp() {
      System.out.println("Usage: [cmd] region key [value]");
      System.out.println("cmd: get/set/evict/quit/exit/help");
   }

}

为什么不用JUnit,TestNG等单元测试框架，居然还用main？

我们想象一下，每次红薯修改完代码，都得认真输入各种命令来验证是不是正确是不是引入了新的BUG？

槽点三：程序结构组织不合理

按照红薯的设计，提供了2级代理：1级是Ehcache，2级是Redis。

那如果我想把Ehcache换成另外一个比如MemCache的话怎么办？

红薯兄冷笑一声，回答：“你的想法我早就想到了，你只要扩充一个新MemcacheCacheProvider”,然后修改j2cache.properties文件中的配置就可以了

说的确实很有道理，但是，你这里依赖进来的Ehcache相关的包怎么办？去一个一个排除吗？

实际上比较好的办法是：

J2Cache工程制定接口标准和框架的实现类，Ehcache和Redis以及其他的缓存实现各自作为独立工程，用的时候依赖进来即可，这样也可以提供不同版本的缓存实现，而不是都在框架底层依赖死。

槽点四：拒绝接受新鲜事物

对于增加程序的灵活性方面，一个是通过依赖注入的方式，一个是通过配置的方式。

当然红薯同学是不屑于引入Spring等依赖注入框架的，也懒得增加配置文件的，所以就导致一个结果，如果你接受，那么就全盘接受我的方案要不就滚犊子你不是我的菜。但是作为一个通用的框架来说这么做应该是远远不够的，这也大大限制了框架的通用型，或者就只能Clone下代码衍生出各种版本来。这个和红薯同学开源的初始目的完全是背离的。

小节

通过上面的分析，由于代码规范性和设计方面的问题，其实已经没有看代码的必要了，但是回想起红薯叫嚣的样子，不行，还得举起大刀继续深深的砍下去：）

代码评审

Main方法

在许多代码里面都有main方法的存在，

在许多安全扫描程序看来，main方法是存在安全问题的，必须加以说明解释或剔除。如果只是用来做一下测试，还是用Unit test case的方式进行比较好，这样每次install deploy release都可以进行测试为你的代码保驾护航，而放在main里面，我敢保证绝大多数的情况下都是不运行的。

从这个角度来说，开发过程管理是存在问题的。

 

解决思路：有些时间看起来是花的多的，比如JUnit测试用例；有些时间看起来是节省的，比如main方法。

实例初始化

private final static CacheProvider getProviderInstance(String value) throws Exception {
   if("ehcache".equalsIgnoreCase(value))
      return new EhCacheProvider();
   if("redis".equalsIgnoreCase(value))
      return new RedisCacheProvider();
   if("none".equalsIgnoreCase(value))
      return new NullCacheProvider();
   return (CacheProvider)Class.forName(value).newInstance();
}

value这个变量名明显有点词不达意，叫cacheType,cacheProviderType是不是好一点?当然其实叫什么不重要，我觉得这里参数用Class<CacheProider> cacheProvider 更好一点，这样一来清晰，而来代码也清晰的多,类似如下：

return cacheProvider.newInstance();

原因如下：第一让使用者记这些魔鬼字符串是不合适的；第二如果扩充新的类型，都要修改核心代码，这里有依赖倒置的问题；第三，你一会儿是类型一会儿是类的名字，你的注释又没有说明，你是准备让使用你的框架的人都必须熟读里面的每一行代码的意思吗？

扩展性的考虑

当我看到红薯提供了配置文件来修改1、2级缓存的实现的时候，说实际我是比较开心，终于有进步了啊，不容易。

不过看到这里的时候：

String cache_broadcast = props.getProperty("j2cache.broadcast");
if ("redis".equalsIgnoreCase(cache_broadcast)) {
   String channel = props.getProperty("redis.channel");
   policy = ClusterPolicyFactory.redis(channel, CacheProviderHolder.getRedisClient());//.getInstance();
}
else if ("jgroups".equalsIgnoreCase(cache_broadcast)) {
   String channel_name = props.getProperty("jgroups.channel.name");
   policy = ClusterPolicyFactory.jgroups(channel_name);//
}
else
   throw new CacheException("Cache Channel not defined. name = " + cache_broadcast);

我才知道，那只是一种错觉，为什么不能完全解耦，透明切换？

修改建议：从Cache核心代码里面忘记Redis&Ehchche，完全不要出现任意一种具体的缓存的名字和相关类。

结论

看到这里的时候，基本上感觉距离成熟还有一段距离，继续努力！

纳尼，居然要夸两句？！好吧，还是要夸的，那就夸几句：

1、感觉红薯还是发挥稳定的，灰常稳定，几年以来代码质量水平非常稳定！！

2、对新的JDK特性了解非常好，充分使用了在接口中实现通用method的方法

3、成功的从V1进化到V2，版本上有巨大越变

4、一如既往的帅，这方面我望尘莫及啊

备注：由于时间和水平的关系，槽点数量和质量肯定有遗漏和不足之处，希望其他Oscer们补充。

本人开源的tinyscript已经成熟，感兴趣的同学请移步查看

 

转载于:https://my.oschina.net/tinyframework/blog/1593035