好得很程序员自学网

<tfoot draggable='sEl'></tfoot>

代码细节重构:请对我的代码指手划脚(二)

代码细节重构:请对我的代码指手划脚(二)

“请对我的代码指手划脚”是我们群内搞的一个不定期的常规性活动,以代码审阅和细节重构为主线,大家可以自由发表自己的意见和建议,也算得上是一种思维风暴。感觉到这个活动很有意义,有必要总结并记录下来。

目标代码

  1   public   static   bool  Serialize(Object obj,  string   fullname)
   2   {
   3      FileStream filestream =  new   FileStream(fullname, FileMode.Create, FileAccess.Write);
   4      BinaryFormatter binaryformatter =  new   BinaryFormatter();
   5  
  6       binaryformatter.Serialize(filestream, obj);
   7  
  8       if  (filestream ==  null  )
   9       {
  10           return   false  ;
  11       }
  12       else 
 13       {
  14           filestream.Close();
  15  
 16           return   true  ;
  17       }
  18  }

看法一——@稻草人

1、没有对传入参数 obj 和 fullname 进行验证。
2、在本段实例代码中,不可能出现filestream 为空的情况,要么调用FileStream会直接抛出异常,中止代码的执行。那么,根据filestream是否为空,来返回true或者false就显得无意义。
3、BinaryFormatter 的 Serialize 倒是有可能会抛出异常,可视函数需要是否捕获该异常。
4、对于实现了IDispose接口的对象,在不使用时,应该显示调用Dispose,以释放资源。

  1   public   static   void  Serialize(Object obj,  string   fullname)
   2   {
   3       if  (obj ==  null )  throw   new  ArgumentNullException( "  obj  "  );
   4       if  ( string .IsNullOrEmpty(fullname))  throw   new  ArgumentNullException( "  fullname  "  );
   5  
  6       using  (FileStream filestream =  new   FileStream(fullname, FileMode.Create, FileAccess.Write))
   7       {
   8          BinaryFormatter binaryformatter =  new   BinaryFormatter();
   9  
 10           binaryformatter.Serialize(filestream, obj);
  11       }
  12  }

看法二——@freeworklife

存在的问题:

1:从程序执行的顺序上说:第6行的判断应该在初始化filestream之后就要判断,这样即使失败了也不用再初始化

BinaryFormatter binaryformatter = new BinaryFormatter(); binaryformatter.Serialize(filestream, obj);

他们了。

2:binaryformatter.Serialize(filestream, obj);

这个是序列化是否成功不知道应该有个判断。

3:没有对传过来的变量进行判断处理。

原因: 没有考虑到意外的情况的发生,考虑不周全,要想把一个函数写好,就要考虑到它可能出现的各种情况,并给与相应的处理,这样函数才是最高效安全实用的。 我个人的建议是: 在写函数时要有个整体的把握前后逻辑要清楚,每条语句都有它功能和可能出现的bug,要全面的考虑才能让函数更健壮。

看法三——@游戏风

1、操作存在大量容易出异常的地方,没有对异常进行处理,比如new FileStream的时候,binaryformatter.Serialize的时候都容易出异常。(filestream == null)
这个判断应该在创建FileStream之后就判断,不然无端的对着空FileStream对象写入序列化数据,铁定要出异常。
2、fullname没有检查是否为合法的路径名称,所以很容易出错,再就是没有进行access权限异常的判断。binaryformatter.Serialize的情况也是没有考虑传入的实例对象是否允许序列化,如果传入一个不能序列化的对象,必然异常。

老陈的看法

缺陷主要有4个:
1、FileStream应当置入using语句;
2、"filestream == null"永远不可能成立!要么抛出异常,反正不会是null;
3、基于第二点"return false;" 永远不会执行!
4、输入参数的检查,至于其他的相对来说不算重要了;

不过,上述三种看法中,有一个问题被忽略或被扭曲了,即方法的返回值是bool,它在项目中需要或已经被他人引用,我们不能在改变这个逻辑的前提下进行重构,因此@稻草人的做法就不妥了。

重构后代码如下:

  1   public   static   bool  Serialize(Object obj,  string   fileName)
   2   {
   3       if  (obj ==  null )  throw   new  ArgumentNullException( "  obj  "  );
   4  
  5       // if (fileName == null) throw new ArgumentNullException("fileName");
   6       if  (String.IsNullOrWhiteSpace(fileName))  throw   new  ArgumentOutOfRangeException( "  fileName  "  );
   7  
  8      FileStream filestream =  null  ;
   9  
 10       try 
 11       {
  12          filestream =  new   FileStream(fileName, FileMode.Create, FileAccess.Write);
  13  
 14           new   BinaryFormatter().Serialize(filestream, obj);
  15       }
  16       catch 
 17       {
  18           return   false  ;
  19       }
  20       finally 
 21       {
  22           if  (filestream !=  null  ) filestream.Close();
  23       }
  24  
 25       return   true  ;
  26  }

原贴地址: http://newsql.cn/thread-81-1-1.html

【IT技术综合群,纯粹.NET技术讨论请入本群】
网鸟-刺客巅峰:47700865 【NoSQL群】NoSQL系列技术QQ群:
一群:23152359(满员)
二群:193713524(接近满员)
三群:79377097(强烈推荐)
四群:191845335(新建)
入群写明理由,否则一律拉黑! 关于老陈:一只不小心踏入互联网十二年的老猴子,喜欢在群里跟大家伙儿研究和分享各种V。天造的懒汉,十足的吃货!

分类:  C# ,  架构设计

 

作者: Leo_wl

    

出处: http://www.cnblogs.com/Leo_wl/

    

本文版权归作者和博客园共有,欢迎转载,但未经作者同意必须保留此段声明,且在文章页面明显位置给出原文连接,否则保留追究法律责任的权利。

版权信息

查看更多关于代码细节重构:请对我的代码指手划脚(二)的详细内容...

  阅读:45次