惯性聚合 高效追踪和阅读你感兴趣的博客、新闻、科技资讯
阅读原文 在惯性聚合中打开

推荐订阅源

让小产品的独立变现更简单 - ezindie.com
让小产品的独立变现更简单 - ezindie.com
U
Unit 42
Google DeepMind News
Google DeepMind News
博客园 - 司徒正美
Y
Y Combinator Blog
F
Fortinet All Blogs
云风的 BLOG
云风的 BLOG
T
Tailwind CSS Blog
G
Google Developers Blog
酷 壳 – CoolShell
酷 壳 – CoolShell
罗磊的独立博客
D
DataBreaches.Net
T
The Blog of Author Tim Ferriss
Cyber Security Advisories - MS-ISAC
Cyber Security Advisories - MS-ISAC
MyScale Blog
MyScale Blog
N
Netflix TechBlog - Medium
Microsoft Security Blog
Microsoft Security Blog
GbyAI
GbyAI
P
Proofpoint News Feed
Jina AI
Jina AI
B
Blog RSS Feed
腾讯CDC
阮一峰的网络日志
阮一峰的网络日志
D
Docker

博客园 - Anders06

A memory leak issue with WPF Command Binding 我也谈面试 附赠一份题目 败给了IEqualityComparer 标注随测量物体旋转问题 How to detect memory leak issue 图纸中角度标注算法 认真的程序员最可爱 避免陷阱,重写Equals方法您需要注意的其中2个原则 What’s the problems with the following code 对象池 关于设计和设计文档的2个补充 SelectMany ConverAll & Exists SOS:利用WPF RichTextBox 提取RTF, 莫名丢掉了Underline样式信息 想探讨两个关于设计文档问题 Don‘t Cry for Me, Argentina 拒绝高姿态 Please avoid implement C# Destruction SOS: How to popup a HwndSource on topmost
Clean Code反案例:什么是不好的API
Anders06 · 2010-08-11 · via 博客园 - Anders06

先来看两段代码:

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应该不用头大了。