Clean Code反案例:什么是不好的API

先来看两段代码:

CI.cs: 

  public BoundBlock3d meshOrthoBoundBlock(Matrix3d localToBound) 
  {
      Matrix3d xfrm = localToBound * this.ChildToParentTransform;
            
return this.Child.meshOrthoBoundBlock(xfrm);
  }

 

CD.cs
代码
public BoundBlock3d meshOrthoBoundBlock(Matrix3d localToBound) 
{
    BoundBlock3d rv = new BoundBlock3d();
    Matrix3d mtx = localToBound * this.ScalingTransform;
    
foreach (ComponentInstance ci in this.ReferencedComponentInstances) 
    {
        rv.orthoExtend(ci.meshOrthoBoundBlock(mtx));
    }

    rv.orthoExtend(this.Body.GetOrthoBoundingBlock(mtx));

     
return rv;
}

类之间的简单关系: CD中包含了CI的几何信息

 

大家可以先看看,仔细想想,如果你一眼看出了问题,那么很是佩服你,你相当的厉害!

 -------------------------------------------------  华丽的分割线  ------------------------------------------------- 

上面这段代码问题有二:

一: 两个方法的修饰符 都是public

这个叫引诱他人犯罪, 因为当外面Client调用CI::meshOrthoBoundBlock(), 你得的结果是错误的,而且很多情况下你还不一定能注意到, 因为它多乘了CI自身的ChildToParentTransform。 你注意到了吗?如果没有理解我说的话,你只需要再研究研究CD里的逻辑。

改进的方法是: 将CI里的public修饰符改成internal, 这样就能防止外面调用CI:meshOrthoBoundBlock(), 他们才会意识到应该调用CD里的方法。

二:Client调用CD::meshOrthoBoundBlock(),得传什么参数好呢?

搜索下可以发现Client都是这么调用的 cd.meshOrthoBoundBlock(Matrix3d.Identity). Oh, shit! 大哥,我怎么知道要传入一个标准矩阵进去啊,即使大家都知道了这个潜规则,这样的接口设计就是脱裤子放屁,多此一举。

改进的方法是:封装一个不带参数的方法供外部调用,并把原先的方法访问修饰符改为internal。  

 

问题一二的引入,我们发现实际上都是为了实现CD中递归计算。 第一个问题看似小问题,却是很需要讲究的,我们必须时刻注意到封装,尽可以封装一切。 对于修饰符的选择,必须使用最严格的,万不要滥用public

下面是我重构后代码,欢迎指正:

CI.cs: 
重构后的CI
public BoundBlock3d MeshOrthoBoundBlock()
{
    
return MeshOrthoBoundBlock(Matrix3d.Identity);
}        

 
private BoundBlock3d MeshOrthoBoundBlock(Matrix3d localToBound)
 {  
     BoundBlock3d bounds = new BoundBlock3d();
     Matrix3d mtx = localToBound * this.Child.ScalingTransform;

     
foreach (CI childCI in this.Child.ReferencedCIs)
       bounds.orthoExtend(MeshOrthoBoundBlock(mtx * childCI.ChildToParentTransform));

     
// Calculate self bounds
     bounds.orthoExtend(activeBody.GetOrthoBoundingBlock(mtx));

     
return bounds;
 }


CD.cs:
   移去meshOrthoBoundBlock方法 

大家可以看到为了避免Client传不必要的Matrix3d.Identity, 我在外面多包装了一层。现在对于外面来说会清净很多,没有混乱的API, Client应该不用头大了。

posted @ 2010-08-11 18:47  Anders06  阅读(284)  评论(0编辑  收藏