一个因参数定义不合理造成的滑稽错误引发的思考

这是一个真实案例，本周在工作中发现的，案例情况比较极端，因此显得很滑稽很搞笑。但是深入一下，还是有些东西值得思考。

    先来看这个案例，在性能优化的过程中，通过thread dump发现有非常多的线程都在执行同一个数据库访问。而按照分析，在cache开启的情况下应该只访问一次才是，后面的数据库访问都是不应该的。

    随即跟踪到问题代码：

    // 1. get pk as method parameter
    public  TrafficProfile createTrafficProfile(
         long  serviceCapabilityPrimaryKey, String serviceProviderId,                             
        String applicationId)  throws  NotFoundException {
 
        //  2. do database query to get serviceCapabilityProfile by pk
        ServiceCapabilityProfile serviceCapabilityProfile  =   new  ServiceCapabilityProfilePreLoadFullSerializableImpl(getContext(),
                serviceCapabilityPrimaryKey);
        //  3. generate key using obj serviceCapabilityProfile
        String key  =  buildTrafficProfileCacheKey(serviceProviderId, applicationId, serviceCapabilityProfile);
        TrafficProfile trafficProfile  =  (TrafficProfile) trafficProfileCache.get(key);
 
        // 5. found in cache and return
         if  ((trafficProfile  !=   null )) {                                                            
             return  trafficProfile;
        }
 
        trafficProfile  =   new  TrafficProfilePreLoadFullSerializableImpl(getContext(), serviceCapabilityProfile,
                serviceProviderId, applicationId);
        trafficProfileCache.put(key, trafficProfile);
         return  trafficProfile;
    }
 
    // 4. notice: in fact only pk is used 
     private  String buildTrafficProfileCacheKey(String serviceProviderId, String applicationId,
            ServiceCapabilityProfile serviceCapabilityProfile) {
         return  serviceCapabilityProfile.getServiceCapabilityPrimaryKey()  +   " , "   +  serviceProviderId  +   " , "                
                 +  applicationId;
    }

    因此可以看到，如果cache有效，我们其实只需要一个pk就可以组合出key从而从cache中得到保存的trafficProfile 对象。但是现在在我们的代码中 ，为了得到key，我们进行了一个从pk -> serviceCapabilityProfile 对象的数据库查询，而在使用这个serviceCapabilityProfile  对象的函数中，很惊讶的发现，其实这里真正用到的不过是一个pk而且，而这个pk我们本来就持有，何须去数据库里跑一回？

pk  ---->   get serviceCapabilityProfile from database by pk  --->  get pk by serviceCapabilityProfile.getServiceCapabilityPrimaryKey();

    让我们来看看为什么会犯下如此可笑的错误，随即在这个类中我们找到了另外一个createTrafficProfile():

  //  parameter is serviceCapabilityProfile obj
public  TrafficProfile createTrafficProfile(
        ServiceCapabilityProfile serviceCapabilityProfile,                                                                     
        String serviceProviderId, String applicationId)
         throws  NotFoundException {
 
        //  pass to buildTrafficProfileCacheKey() is obj, not pk
        String key  =  buildTrafficProfileCacheKey(serviceProviderId, applicationId, serviceCapabilityProfile);                   

    现在原因就很清楚了：在方法buildTrafficProfileCacheKey()中，实际只需要一个long类型的pk值，但是在它的方法参数定义中，它却要求传入一个serviceCapabilityProfile 的对象。

    可以想象一下这个代码开发的过程：

1. 第一个人先增加了以serviceCapabilityProfile对象为参数的createTrafficProfile()方法
2. 他创建了buildTrafficProfileCacheKey()方法，因为手头就有serviceCapabilityProfile对象，因此他选择了将整个对象传入
3. 这两个函数工作正常，虽然这个参数传递的有点感觉不大好，但至少没有造成问题

4. 后来，另外一个人来修改这个代码，他添加了使用long serviceCapabilityPrimaryKey的createTrafficProfile()方法
5. 他试图调用buildTrafficProfileCacheKey()方法，然后发现这个方法需要一个serviceCapabilityProfile 对象
6. 他不得不进行一次数据库访问来获取整个对象数据......
 
    从这个案例中，我们可以看到，一个含糊的参数是如何导致我们最终犯错的 ^0^

    这个错误的修改当然非常简单，将buildTrafficProfileCacheKey()方法的参数调整为传入long类型的pk就解决了问题。
 
    在日常代码中，我们有非常多的大对象诸如“****DTO/context/profile”，而它们经常被作为参数在代码之间传递。因此需要小心：
 
1. 当定义一个类似buildTrafficProfileCacheKey()的方法时
    尽量将接口的参数简单化，如果我们确认只是需要使用到某个大对象的一两个简单属性，请将方法定义为简单类型，不需要传入整个对象。
    或者在方法上通过javadoc说明我们只需要这个对象的某个或某几个属性。

2. 当调用类似buildTrafficProfileCacheKey()的方法时
    需要稍微谨慎一些，进去目标方法，看看代码实现，到底是需要什么数据，是否真的需要整个对象从而导致我们需要进行数据库查询这种的重量级操作。
    例如上面的例子，如果原有buildTrafficProfileCacheKey()的方法不容许修改，那么我们大可以new 一个serviceCapabilityProfile 对象，然后setPK()来解决，比访问数据库快捷多了。

    前面提到说这个案例有点"极端"，这里的极端指的是buildTrafficProfileCacheKey()方法本身就在这个类之中，代码量也非常少，意图非常明确，本来应该很容易被发现的。因此犯错的情况显得比较可笑，但是我们推开来想一想，问题似乎没有这么简单了：如果buildTrafficProfileCacheKey()中的代码比较复杂，可能还通过调用其他的类从而将对serviceCapabilityProfile对象的时候的代码逻辑转移，恶劣的情况下可能还有多层调用，甚至出现接口抽象实际代码运行时注入等复杂场景，再假设我们没有办法直接看到最终的使用代码，我们无法知道原来底层只是需要一个pk而已！那么这个问题就一点都不可笑，上面这个白白访问一次数据库的错误一定会再次发生，因为上层调用者不知道到底需要什么数据，只好整个对象全给！何况通常上层都有良好的代码封装，通过一个pk获取一个对象这种事情，可能只需要一两行代码调用就搞定，于是我们很可能轻松自如的，一脚踩进坑里！

    所以说想复杂点问题就变得严峻起来：底层代码的实现者，需要如何设计接口参数，才能准确的告知上层调用者，到底哪些数据是真实需要的？上面的案例中将参数简单的简化为只传入一个pk值就明确的达到了目标，对调用者来说足够清晰明确。但是我们考虑一下复杂场景：如果底层的实现逻辑没有这么简单明确，底层代码的实现者可能担心未来的实现逻辑会发生更改，比如需要serviceCapabilityProfile的其他数据，因此为了保持接口稳定，底层代码的实现者一定会倾向于使用serviceCapabilityProfile对象作为参数从而保留未来不需要修改接口/函数定义就可以扩展的自由。不经意间，挖了一个坑...

    我们似乎又回到了原来犯错的轨道中，那个看似搞笑的错误似乎又在对我们挥手微笑......
    只是现在，我颇有点笑不起来了：下一次，如果我面对一个函数/接口，要求传入一个大对象，我手头只有一个pk，还有一个现成的函数可以一行代码就搞定查询，我要如何才能挡住诱惑？