开发者

C# Antipatterns

Locked. This question and its answers are locked because the question is off-topic but has historical significance. It is not currently accepting new answers or interactions.

To cut a long story short: I 开发者_C百科find the Java antipatterns an indispensable resource. For beginners as much as for professionals. I have yet to find something like this for C#. So I'll open up this question as community wiki and invite everyone to share their knowledge on this. As I am new to C#, I am strongly interested in this, but cannot start with some antipatterns :/

Here are the answers which I find specifically true for C# and not other languages.

I just copy/pasted these! Consider throwing a look on the comments on these as well.


Throwing NullReferenceException

Throwing the wrong exception:

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

Properties vs. public Variables

Public variables in classes (use a property instead).

Unless the class is a simple Data Transfer Object.


Not understanding that bool is a real type, not just a convention

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

or, even better

if (myBooleanVariable != false)
{
    ...
}

Constructs like these are often used by C and C++ developers where the idea of a boolean value was just a convention (0 == false, anything else is true); this is not necessary (or desirable) in C# or other languages that have real booleans.


Using using()

Not making use of using where appropriate:

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.


Rethrowing the exception incorrectly. To rethrow an exception :

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
}


GC.Collect() to collect instead of trusting the garbage collector.


I see this one way too much, both in Java and C#...

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

with bonus points if it also has

else{
  somethingelse = false;
}


using Microsoft.SharePoint;

'nuff said


I see following code a lot:

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

should be:

       return (i==3);


Insulting the law of Demeter:

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


Throwing NullReferenceException:

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


This is true I seen it with my own eyes.

public object GetNull()
{
     return null;
}

It was actually used in the app, and even had a stored procedure to go with it too, an sp_GetNull that would return null....

that made my day.

I think the sp was used for a classic asp site .. something to do with a result set. the .net one was someone idea of "converting" the code into .net...


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

Or the more general case:

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


I have found this in our project and almost broke the chair...

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


Quite often I stumble over this kind of var-abuse:

var ok = Bar();

or even better:

var i = AnyThing();

Using var that way makes no sense and gains nothing. It just makes the code harder to follow.


  • Lack of null test before delegate invocation.
  • Not knowing when and how to use 'as' with a null check vs. a cast with a try/catch.
  • 'throw exception' vs. 'throw' within a catch block.
  • Instantiating a large number of strings instead of using StringBuilder.
  • Deep nesting of using blocks.


Not understanding that bool is a real type, not just a convention

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

or, even better

if (myBooleanVariable != false)
{
    ...
}

Constructs like these are often used by C and C++ developers where the idea of a boolean value was just a convention (0 == false, anything else is true); this is not necessary (or desirable) in C# or other languages that have real booleans.

Updated: Rephrased the last paragraph to improve its clarity.


Public variables in classes (use a property instead).

Unless the class is a simple Data Transfer Object.

See comments below for discussion and clarification.


I have actually seen this.

bool isAvailable = CheckIfAvailable();
if (isAvailable.Equals(true))
{ 
   //Do Something
}

beats the isAvailable == true anti-pattern hands down!
Making this a super-anti-pattern!


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.


Private auto-implemented properties:

private Boolean MenuExtended { get; set; }


Declaring and initializing all local variables at the top of each method is so ugly!

void Foo()
{
    string message;
    int i, j, x, y;
    DateTime date;

    // Code
}


Two string anti patterns
Anti-Pattern # 1
Checking strings for null or empty

//Bad
if( myString == null || myString == "" )
OR
if( myString == null || myString.Length == 0 )

//Good
string.IsNullOrEmpty(myString)

Anti-Pattern # 2 (only for .NET 4.0)
Checking strings for null or empty or white space

//Bad
if( myString == null || myString == "" || myString.Trim() == "")

//Good
string.IsNullOrWhiteSpace(myString) 


Needless casting (please trust the compiler):

foreach (UserControl view in workspace.SmartParts)
{
  UserControl userControl = (UserControl)view;
  views.Add(userControl);
}


if(data != null)
{
  variable = data;
}
else
{
  variable = new Data();
}

can be better written as

variable = (data != null) ? data : new Data();

and even better written as

variable = data ?? new Data();

Last code listing works in .NET 2.0 and above


Speaking with an accent always caught me.

C++ programmers:

if (1 == variable) { }

In C# this will give you a compiler error if you were to type if (1 = variable), allowing you to write the code the way you mean it instead of worrying about shooting yourself in the foot.


Not using ternary's is something I see converts to c# do occasionally

you see:

private string foo = string.Empty;
if(someCondition)
  foo = "fapfapfap";
else
  foo = "squishsquishsquish";

instead of:

private string foo  = someCondition ? "fapfapfap" : "squishsquishsquish";


Accessing modified closures

foreach (string list in lists)
{
        Button btn = new Button();
        btn.Click += new EventHandler(delegate { MessageBox.Show(list); });
}

(see link for explanation and fix)


For concating arbitrary number of strings using string concatenation instead of string builder

Exampls

foreach (string anItem in list)
    message = message + anItem;


is this considered general ?

public static main(string [] args)
{
  quit = false;
  do
  {
  try
  {
      // application runs here .. 
      quit = true;
  }catch { }
  }while(quit == false);
}

I dont know how to explain it, but its like someone catching an exception and retrying the code over and over hoping it works later. Like if a IOException occurs, they just try over and over until it works..


The project I'm on had fifty classes, all inheriting from the same class, that all defined:

public void FormatZipCode(String zipCode) { ... }

Either put it in the parent class, or a utility class off to the side. Argh.

Have you considered browsing through The Daily WTF?


Massively over-complicated 'Page_Load' methods, which want to do everything.


Using properties for anything other than to simply retrieve a value or possibly an inexpensive calculation. If you are accessing a database from your property, you should change it to a method call. Developers expect that method calls might be costly, they don't expect this from properties.


Found this a few times in a system I inherited...

if(condition){
  some=code;
}
else
{
  //do nothing
}

and vice versa

if(condition){
  //do nothing
}
else
{
  some=code;
}
0

上一篇:

下一篇:

精彩评论

暂无评论...
验证码 换一张
取 消

最新问答

问答排行榜