C# 反模式

55
简而言之:我认为Java反模式是一份不可或缺的资源,无论对于初学者还是专业人士。但是我尚未找到类似于C#的内容。因此,我将开放这个问题作为社区wiki,并邀请每个人分享他们在这方面的知识。由于我刚开始接触C#,所以我非常感兴趣,但不能从一些反模式开始 :/

以下是我认为特别适用于C#而不适用于其他语言的答案。

我只是复制/粘贴了这些内容!也可以看看这些评论。


抛出NullReferenceException

抛出错误的异常:

if (FooLicenceKeyHolder == null)
    throw new NullReferenceException();

属性 vs. 公共变量

在类中使用公共变量时,请改用属性。

除非该类是一个简单的数据传输对象。


没有意识到bool是一种真实类型,而不仅仅是一种约定

if (myBooleanVariable == true)
{
    ...
}

或者,更好的方法是:
if (myBooleanVariable != false)
{
    ...
}

像这样的结构经常被 CC++ 开发人员使用,其中布尔值的概念只是一种惯例(0 == false,其他任何值都是true);在C#或其他具有真正布尔类型的语言中,这是不必要的(也不可取的)。


使用 using()

没有适当地使用 using

object variable;
variable.close(); //Old code, use IDisposable if available.
variable.Dispose(); //Same as close.  Avoid if possible use the using() { } pattern.
variable = null; //1. in release optimised away.  2. C# is GC so this doesn't do what was intended anyway.

2
好吧...这正是我试图避免的。作为“重复”的链接,该问题包含许多常见的OO“不良实践”。我已经开发了10多年了,这些对我来说并不新鲜。在这种情况下,我期望的是特定的**C#**反模式。 - exhuma
3
您提供的Java“反模式”实际上并不是反模式(例如低效和/或产生反效果的设计模式),而是糟糕的编码实践,与您问题的大多数答案一样。像设计模式一样,反模式与语言无关。 - comichael
好观点。如果考虑“设计模式”,有些是与语言无关的,但是有些在某种语言中更有意义。如果语言本身针对给定问题提供了解决方案,则其他可能与该语言无关。因此,我认为您也可以找到比其他语言更相关的C#反模式。一遍又一遍地阅读相同的好/坏做法变得很无聊;) - exhuma
Dispose并不总是包括Close。在大多数.NET类中,它是包含在内的,并且这是常识,但它并没有明确暗示。 - Michael Stum
38个回答

62

错误地重新抛出异常。要重新抛出异常:

try
{
    // do some stuff here
}
catch (Exception ex)
{
    throw ex;  // INCORRECT
    throw;     // CORRECT
    throw new Exception("There was an error"); // INCORRECT
    throw new Exception("There was an error", ex); // CORRECT
}

31
"catch (Exception ex)"本身就是一种反模式。 - Joren
除非你正在进行一些日志记录或类似的操作,然后重新抛出它。 - thecoop
18
@Joren,没有throw的catch (Exception ex)是一种反模式... 如果你会抛出异常(例如记录日志、抛出新的异常类型、回滚某些资源等),那么捕获通用异常类型会有很多有效的情况。但当然,规则因应用程序、库、插件等而异。 - Pop Catalin
在 Web 应用程序中,catch (Exception ex) 是完全有效的。可能还有其他情况。 - Egor Pavlikhin
aaah throw new Exception("发生错误", ex); 这样它就会显示原始异常作为内部异常,很好! - Carlo
5
如果您想隐藏原始错误(例如库中的错误)并且可能与安全有关,则throw new Exception("There was an error")可能是完全有效的。 - erikkallen

40

GC.Collect()用于垃圾回收器不可靠时进行垃圾收集。


12
如果你确实想要强制进行垃圾回收,但未等待挂起的终结器完成,这种情况很少见。 - Eric Lippert
4
如果出现了碎片化引用或者请求的内存超过系统可用内存,你就会遇到OutOfMemoryException异常。你确定你的页面文件大小是固定的吗?如果你认为你能写出比Microsoft更好的垃圾回收器,那就试试吧,然后让他们从你这里购买。 - Matthew Whited
6
在典型的应用程序中,GC.Collect() 几乎没有有效的使用情况。你需要有一个特殊的应用场景才能调用.NET 的 GC.Collect() 方法(例如游戏引擎,在加载关卡后调用该方法,这样 GC 就不会在关卡进行中开始执行全面回收,收集先前分配的对象),或者在近实时应用程序中,其中需要对 GC 行为进行控制和预测,或者在服务器应用程序中,当你知道会空闲一段时间并利用这个机会进行垃圾回收。 - Pop Catalin
@Pop Catalin -- 所有有效的用例...什么是“正常应用程序”?对我来说,所有这些都像是相当“正常”的应用程序。 - Robert Fraser
@Robert Fraser,我从未使用过“正常”这个词,而是用“典型”的意思,指的是现在.NET开发人员日常编写的内容,但未来情况会改变,“典型”将包括比现在更广泛的范围。 - Pop Catalin
显示剩余4条评论

30

我在 Java 和 C# 中经常看到这个问题...

if(something == true){
  somethingelse = true;
}

如果还有额外的一些点数,那就更好了

else{
  somethingelse = false;
}

4
没有奖金的话(只有"== true"),情况就不是那么糟糕。 - ripper234
9
考虑第一个情况,假设something = false,而somethingelse = true。你不应该将其重构为somethingelse = something。 - vanja.
除了这个冗长之外,它并不算是一个“bug”。 - Johnno Nolan
它比必要的更冗长,但除此之外,没有任何问题。 - Joh
@Robert Fraser:啊,我在想C语言。一个更合适的(但在C#中不存在)是||=。或者长手写法表示为:somethingelse = something || somethingelse。(当然,这假设两件事情:somethingelse有一个值,并且没有使用else;否则,somethingelse = something就足够了/)。 - Tordek
显示剩余4条评论

25
using Microsoft.SharePoint;

'nuff said


22

我经常看到以下代码:

if (i==3)
       return true;
else
       return false;

应该是:

       return (i==3);

是的,我经常看到这种情况。如果有时间,我总是会重构它。 - rein
这里真正糟糕的是缺少括号 :) - Egor Pavlikhin
缺少括号?你一定是在指编码风格的偏好。 - Robert Harvey
29
return 不是一个函数,括号是不必要的。 - P Daddy
7
@DisgruntledGoat:我觉得它们看起来像是噪音。 - P Daddy
显示剩余2条评论

17

侵犯了迪米特法则:

a.PropertyA.PropertyC.PropertyB.PropertyE.PropertyA = 
     b.PropertyC.PropertyE.PropertyA;

+1,但不幸的是,在某些情况下(特别是在静态类中),它似乎很难摆脱...例如,最近我写了这个:this.Left = Screen.AllScreens[target].Bounds.X - Matthew Scharley
@matthew:让我困扰的不是左侧,而是赋值/变异问题。你能告诉我你正在修改什么吗? - leppie
我正在将我的表单左侧设置为目标屏幕的左边缘。实际上,Screen.AllScreens 被别名为本地变量,因为我不想在每个地方都输入完整的限定符,但这只是缩短了我要输入的内容,而没有改变意图。 - Matthew Scharley

17

抛出 NullReferenceException 异常:

if (FooLicenceKeyHolder == null)
    throw new NullReferenceException();

2
此外,我认为一般的反模式是在发生错误时手动抛出运行时本身抛出的任何异常。 - Matthew Scharley
是的,但这个特别具有误导性,因为那不是真正发生的事情。 - Ed S.
5
从代码中看,似乎这个ArgumentNullException并不是合适的方式,因为this.FooLicenseHolder并不像是函数的参数(至少从命名规范上看不像)。也许稍微重新表达一下代码会使你的(完全正确的)观点更清晰一些。 - petr k.
@petr:你说得没错。这是属性,而不是参数。这使得代码变得更加可怕! - leppie
扩展方法怎么样?使用NullReferenceExceptions会不会是一个好的选择呢? - Andrei Rînea
显示剩余2条评论

16

这是真的,我亲眼见过。

public object GetNull()
{
     return null;
}

这实际上在应用程序中被使用了,甚至还有一个存储过程与之配套,名为sp_GetNull,它会返回null....

那真是让我开心啊。

我认为这个存储过程被用于经典的asp网站...与结果集有关。.net版本是将代码“转换”成.net的某人的想法...


2
好的,你可以将空值的概念封装在一个函数中 :P - Daniel Daranas
7
他们一定认为空值可能会在未来发生变化 :) - PeteT

14
int foo = 100;
int bar = int.Parse(foo.ToString());

或者更一般的情况:

object foo = 100;
int bar = int.Parse(foo.ToString());

9
这……什么?为什么?-摔桌- 你不可能告诉我是有人做了这个? - Matthew Scharley
@matthew:添加了一个更一般化的情况(从数据表中提取,嘿嘿) - leppie
2
说真的,我见过很多人为整型数据做这种事情,这只是我经历过的诸多令人费解的时刻之一。 - Andrew Barrett
@Carlo:这真的是非常好的实践;1.ToString()将使用区域设置,以防默认情况下1通常拼写为其他内容 :) (也许他们会得到数字的UNICODE“智能”版本,就像MS Word喜欢在您不要求时给您他们的“智能”引号一样) - sehe
@sehe 哈哈哈,那就没有什么好尴尬的了! - Carlo
显示剩余6条评论

12

我在我们的项目中发现了这个问题,几乎把椅子摔烂了...

DateTime date = new DateTime(DateTime.Today.Year, 
                             DateTime.Today.Month, 
                             DateTime.Today.Day);

7
记录一下,正确使用方法是:DateTime date = DateTime.Now; - Wedge
8
或者 DateTime date = DateTime.Today; - Adriaan Stander
12
实际上,正确的用法是:DateTime date = DateTime.Now.Date;(或Today.Date)。如果省略了Date,你将得到时间和日期。 - Meta-Knight
6
我看不出这段代码有什么大问题,也许编写者只是不知道"Date"属性而已... -1 - Meta-Knight
3
嗯,正确的用法是将日期赋值给 Datetime.Now.Date - Spence
显示剩余6条评论

网页内容由stack overflow 提供, 点击上面的
可以查看英文原文,
原文链接