前些日子对公司所有.net产品使用Klocwork进行了静态代码分析。由于Klocwork目前没有发现有破解版,虽然可用于C++、Java、C#等项目,但因价格不菲,其使用细节我就不多讲。通过这次对代码的静态分析我们还是发现了一些问题,有些可能是我们本身不太认真直接造成的;还有一些则可能是由于代码的在长期维护或是后期的不断重构中,间接的造成了相关问题。下面我们就一些比较常见的现象进行罗列。
[其中有一些项与VS自带的代码分析相同,代码在VS2010+.net3.5+Win7 环境中测试通]
一、值类型
1、 浮点数据比较
double x1 = 0.1;
float x2 = 0.1f;
if (x1 == x2) //浮点数的比较,不能直接等于或不等于,此处不相等。
{
}
浮点数的比较,有他特定的规则,即:误差在规定的范围内。但在现实中,我们总是很懒,容易简单的写成 if (a == b)。
2、 空值判断
int i = 0;
//…
if (i == null) //值类型,此处永远不为空。即此处永远为false。
{
}
3、 获取Object.ReferenceEquals
object obj = new object();
int i = 5;
bool res = Object.ReferenceEquals(i, obj); //这样的判断永远为false,无意义。
4、 String类型“+”的问题
string res = string.Empty;
int count = 100;
for (int i = 0; i < count; i++) //foeach 中也适应。
{
res += "Hello"; //效率低,建议改用StringBuilder进行操作。
}
5、 整型数据除后还是整形,精度丢失
int a = 11;
long b = 5; //int b=5; 结果也一样。
decimal d = a / b; //想要的结果为2.5,其实d值为2。
float f = a / b; //想要的结果为2.5,其实f值为2.0。
二、无意义的判断
1、 do操作
int x = 0;
do
{
x++;
//...
} while (3 < 10); // 条件是常数,永远为true,且表达式无意义、可读性差。
//也适应于类似:int m = 3; int n = 10; while (m < n)的情况,但m、n自始至终都没有改变过。
//建议改为类似如下方式。
do
{
x++;
//...
} while (true);
2、 if操作
if (3 < 10) //无意义,永远为true。
{
}
3、 swich操作
switch (10 - 3) //无意义,永远匹配7
{
case 7:
break;
//……
}
4、 ?操作
return ((10 - 3) > 1) ? 0 : 1; //无意义,永远返回0
5、 While操作
while (false) //此处while内的操作永不执行,无意义。也适应于类似While (10 < 3)
{
// ...
}
三、空指针
预先一段代码,为方便说明:
/// <summary> /// 示例类.只为说明配置项,无具体代表意义 /// </summary> public class Example { public Example() { Init(); } protected virtual void Init() { Debug.WriteLine("Init Example"); } #region other /// <summary> /// 随机创建一个Example对象,可能为实例,也可能为null /// </summary> /// <returns>返回一个对象,或返回null</returns> public static Example Factory() { Random random = new Random(); int randKey = random.Next(0, 2); if (randKey == 0) { return null; } else { return new Example(); } } /// <summary> /// 随机产生一个标志,要能为true也可能为false. /// </summary> /// <returns></returns> public static bool GetFlag() { Random random = new Random(); int randKey = random.Next(0, 2); if (randKey == 0) { return false; } else { return true; } } /// <summary> /// 如果example为null,那么必定引发异常 /// </summary> /// <param name="example"></param> /// <returns></returns> public static bool CheckName(Example example) { if (string.IsNullOrEmpty(example.Name)) { return true; } else { return false; } } /// <summary> /// 如果example为null,可以引发异常 /// </summary> /// <param name="example"></param> /// <param name="flag"></param> /// <returns></returns> public static bool CheckData(Example example, bool flag) { if (!flag) { return false; } if (example.Data < 0) { return false; } else { return true; } } public string Name { get; set; } public int Data { get; set; } #endregion }
1、 有机会调用可能为null的对象
Example example = Example.Factory();
bool flag = Example.GetFlag();
if (flag)
{
Example.CheckName(example); //如果exaple为null,且flag为true,则会引发异常
//以下代码是CheckName内部
//if (string.IsNullOrEmpty(example.Name)) //如果传入的为null对象,则exaple.Name必引发异常
//{
// return true;
//}
//else
//{
// return false;
//}
}
2、 调用可能为null的对象
Example example = Example.Factory();
Example.CheckName(example); //如果exaple为null,则会引发异常
//以下代码是CheckName内部
//if (string.IsNullOrEmpty(example.Name)) //如果传入的为null对象,则exaple.Name必引发异常
//{
// return true;
//}
//else
//{
// return false;
//}
3、 还有许多关于传入空引用或可能传入空应用,在此也不详细说明,大同小异。重点想与大家探讨 一下如下疑惑:
Example example = Example.Factory();
//方案1:如果返回的结果可能为空,那么就忽必对其为空时进行判断
if (example != null)
{
Example.CheckName(example);
}
//方案2:对CheckName(example)函数内部进行有效性判断
if (example == null)
{
return true; //或者返回其它
}
if (string.IsNullOrEmpty(example.Name))
{
return true;
}
else
{
return false;
}
疑惑:
①、我个人觉得,如果方案1和方案2中的代码都有进有效性判断应该是比较严谨的。
②、但是人总是有惰性的,比如方案2中,每写一个函数都得先对其输入的对象参数是否为null进行判断,总觉有点繁琐。
③、就我个人在编程时,对方案1的实施我还是比较到位,因为可能为空的对象我还是尽量进行判断。 因为不敢肯定别人提供的Example.CheckName(example)是否进行了有效性判断。在对方案2的实施确较差,除非这些功能是提供给别人使用的,如果是自己用却少有判断,看了这个后,我是是否该改一改了?
④、也不知大家是否有其它方案或习惯?请传授一下。
四、变量被覆盖问题
如果一个类有个成员变量为:
private int price;
1、 变量被参数覆盖
public void InitPrice(int price)
{
this.price = price;
}
2、 变量被临时变量覆盖
public void DoSomeThing()
{
int price = 0;
//....
this.price = price;
}
3、 成员变量如此,属性也一样。不过由于C#常用Pascal命名方法所以重名较少。
疑惑:
①、由于采用Passcal命名方式,在考虑简洁、可读性强的情况下很容易出现重名的现象,至少我的代码中就有样的情况。有时我也会采用类似“currentPrice”来作为成员变量,但终究不能适应所有场合。
②、每当这个时候就会想起匈牙利命名方法来个“m_price”。但是在很多公司,至少我这里是这样,命名方式不能混搭,所以有时就有点为难了。
③、其实在被覆盖后,不经意间就会把“this.”给忘了,特别在复制粘贴时更甚,比如从某未被覆盖处拷贝“price = GetMaxPrice();”到被覆盖处,这样就造成了错误或未达到效果,这也是提示“警告”的原因吧。
④、不知大家会不会有这方面的考虑,有无更好的解决方案?
五、强制类型转换
1、 强制转换类
在C#中由于有is和as两个关键字,相对来说如下代码较少发生。
Example example = null;
object obj = new object();
example = (Example)obj; //类型不匹配会引发异常。
SimpleExample simpleExample = (SimpleExample)example;
2、 循环中未经判断直接类型转换
ArrayList list = new ArrayList();
list.Add(1);
list.Add(2);
list.Add(3);
int value = 0;
foreach (int i in list) //也许list是我们自己构造的,所以我们并不担心,但并不严谨
{
value += i;
}
//某一天,有人重构
list.Add("全部");
//…
foreach (int i in list) //必产生异常
{
}
注:虽然ArrayList现在用的少,但此问题确是存在,比如常见的有如“下拉框”中的项等。
3、 Landa表达式中
List<Example> exampleList = new List<Example>();
//...
var temp = exampleList.Where(p => p.Name.Length > 5).ToList(); //此处警告
疑惑:
本人不解未发现异样?(不会是误报吧?)
六、其它:
1、 构造函数中调用虚函数
由于子类在初始化时,会调用基类的初始化函数。然而由于多态的原因,在调用基类初始化函数时,调用的那个虚函数却又是子类的函数。从而给人以混乱的感觉,也可能达不到预期的效果。我所在的公司《C#编程规范》中明确要求构造函数不允许调用虚函数。
来看个示例吧,代码如下:
/// <summary> /// 示例类.只为说明配置项,无具体代表意义 /// </summary> public class Example { public Example() { Init(); } protected virtual void Init() { Console.WriteLine("Init Example"); } } /// <summary> /// 子类 /// </summary> public class SimpleExample : Example { public SimpleExample() { Init(); } protected override void Init() { Console.WriteLine("Init SimpleExample"); } }
SimpleExample simpleExample = new SimpleExample();
输出结果了并非意想的:
Init Example
Init SimpleExample
而是:
为什么会这样,直接上图吧:
但F11后“奇迹”出现了,感觉也混乱了!
2、 Catch中无操作
当Catch中无操作时,会发出警告。园子中精华区有个“throw和throw ex的区别”讲得不错,可解决此问题,有兴趣的朋友可以看看。当然在软件系统中如何throw异常,在对象设计及函数设计时如何throw异常信息(主要是信息)也可能各不尽然,限于篇幅在此不做详述,以后归纳一下再与大家交流。
3、 资源释放
//1、非托管资源
string fileNaem = "C:\\1.txt";
StreamReader reader = new StreamReader(fileNaem);
String result = reader.ReadToEnd(); //如果此处抛出异常,那么Close不会调用,文件被长时间点用
reader.Close();
//2、托管资源
Form testForm = new Form();
testForm.ShowDialog(); //如果Form中没有非托管资源,应该可以回收。建议也使用using
个人观点:
①、对于非托管资源,如果出现异常不能正常释放,那么就会长时间占用资源,造成不良后果。但是对于托管资源,可能影响不大。
②、对实现了IDisposable的类,建议使用using来管理。
③、对于其它资源,可以通过try- finally来管理,即在finally中对资源进行释放。