About C#

C# is one of several languages that target the Microsoft Common Language Runtime (CLR). Languages that target the CLR benefit from features such as cross-language integration and exception handling, enhanced security, a simplified model for component interaction, and debugging and profiling services.  Of today’s CLR languages, C# is the most widely used for complex, professional development projects that target the Windows desktop, mobile, or server environments.

C# is an object oriented, strongly-typed language. The strict type checking in C#, both at compile and run times, results in the majority of typical programming errors being reported as early as possible, and their locations pinpointed quite accurately. This can save the programmer a lot of time, compared to tracking down the cause of puzzling errors which can occur long after the offending operation takes place in languages which are more liberal with their enforcement of type safety.  However, a lot of programmers unwittingly (or carelessly) throw away the benefits of this detection, which leads to some of the issues discussed in this article.

About this article

This article describes 10 of the most common programming mistakes made, or pitfalls to be avoided, by C# programmers.

While most of the mistakes discussed in this article are C# specific, some are also relevant to other languages that target the CLR or make use of the Framework Class Library (FCL).

Common Mistake #1: Using a reference like a value or vice versa

Programmers of C++, and many other languages, are accustomed to being in control of whether the values they assign to variables are simply values or are references to existing objects. In C#, however, that decision is made by the programmer who wrote the object, not by the programmer who instantiates the object and assigns it to a variable.  This is a common “gotcha” for newbie C# programmers.

If you don’t know whether the object you’re using is a value type or reference type, you could run into some surprises. For example:

  Point point1 = new Point(20, 30);
  Point point2 = point1;
  point2.X = 50;
  Console.WriteLine(point1.X);       // 20 (does this surprise you?)
  Console.WriteLine(point2.X);       // 50
  
  Pen pen1 = new Pen(Color.Black);
  Pen pen2 = pen1;
  pen2.Color = Color.Blue;
  Console.WriteLine(pen1.Color);     // Blue (or does this surprise you?)
  Console.WriteLine(pen2.Color);     // Blue

As you can see, both the Point and Pen objects were created the exact same way, but the value of point1 remained unchanged when a new X coordinate value was assigned to point2, whereas the value of pen1 was modified when a new color was assigned to pen2. We can therefore deduce that point1 and point2 each contain their own copy of a Point object, whereas pen1 and pen2 contain references to the same Pen object. But how can we know that without doing this experiment?

The answer is to look at the definitions of the object types (which you can easily do in Visual Studio by placing your cursor over the name of the object type and pressing F12):

  public struct Point { … }     // defines a “value” type
  public class Pen { … }        // defines a “reference” type

As shown above, in C#, the struct keyword is used to define a value type, while the class keyword is used to define a reference type. For those with a C++ background, who were lulled into a false sense of security by the many similarities between C++ and C# keywords, this behavior likely comes as a surprise.

If you’re going to depend on some behavior which differs between value and reference types – such as the ability to pass an object as a method parameter and have that method change the state of the object – make sure that you’re dealing with the correct type of object.

Common Mistake #2: Misunderstanding default values for uninitialized variables

In C#, value types can’t be null. By definition, value types have a value, and even uninitialized variables of value types must have a value. This is called the default value for that type.  This leads to the following, usually unexpected result when checking if a variable is uninitialized:

  class Program {
      static Point point1;
      static Pen pen1;
      static void Main(string[] args) {
          Console.WriteLine(pen1 == null);      // True
          Console.WriteLine(point1 == null);    // False (huh?)
      }
  }

Why isn’t point1 null? The answer is that Point is a value type, and the default value for a Point is (0,0), not null. Failure to recognize this is a very easy (and common) mistake to make in C#.

Many (but not all) value types have an IsEmpty property which you can check to see if it is equal to its default value:

  Console.WriteLine(point1.IsEmpty);        // True

When you’re checking to see if a variable has been initialized or not, make sure you know what value an uninitialized variable of that type will have by default and don’t rely on it being null..

Common Mistake #3: Using improper or unspecified string comparison methods

There are many different ways to compare strings in C#.

Although many programmers use the == operator for string comparison, it is actually one of the least desirable methods to employ, primarily because it doesn’t specify explicitly in the code which type of comparison is wanted.

Rather, the preferred way to test for string equality in C# is with the Equals method:

  public bool Equals(string value);
  public bool Equals(string value, StringComparison comparisonType);

The first method signature (i.e., without the comparisonType parameter), is actually the same as using the == operator, but has the benefit of being explicitly applied to strings. It performs an ordinal comparison of the strings, which is basically a byte-by-byte comparison. In many cases this is exactly the type of comparison you want, especially when comparing strings whose values are set programmatically, such as file names, environment variables, attributes, etc. In these cases, as long as an ordinal comparison is indeed the correct type of comparison for that situation, the only downside to using the Equals method without a comparisonType is that somebody reading the code may not know what type of comparison you’re making.

Using the Equals method signature that includes a comparisonType every time you compare strings, though, will not only make your code clearer, it will make you explicitly think about which type of comparison you need to make. This is a worthwhile thing to do, because even if English may not provide a whole lot of differences between ordinal and culture-sensitive comparisons, other languages provide plenty, and ignoring the possibility of other languages is opening yourself up to a lot of potential for errors down the road.  For example:

  string s = "strasse";
  
  // outputs False:
  Console.WriteLine(s == "straße");
  Console.WriteLine(s.Equals("straße"));
  Console.WriteLine(s.Equals("straße", StringComparison.Ordinal));
  Console.WriteLine(s.Equals("Straße", StringComparison.CurrentCulture));        
  Console.WriteLine(s.Equals("straße", StringComparison.OrdinalIgnoreCase));
  
  // outputs True:
  Console.WriteLine(s.Equals("straße", StringComparison.CurrentCulture));
  Console.WriteLine(s.Equals("Straße", StringComparison.CurrentCultureIgnoreCase));

The safest practice is to always provide a comparisonType parameter to the Equals method. Here are some basic guidelines:

  • When comparing strings that were input by the user, or are to be displayed to the user, use a culture-sensitive comparison (CurrentCulture or CurrentCultureIgnoreCase).
  • When comparing programmatic strings, use ordinal comparison (Ordinal or OrdinalIgnoreCase).
  • InvariantCulture and InvariantCultureIgnoreCase are generally not to be used except in very limited circumstances, because ordinal comparisons are more efficient.  If a culture-aware comparison is necessary, it should usually be performed against the current culture or another specific culture.

In addition to the Equals method, strings also provide the Compare method, which gives you information about the relative order of strings instead of just a test for equality. This method is preferable to the <, <=, > and >= operators, for the same reasons as discussed above.

Common Mistake #4: Using iterative (instead of declarative) statements to manipulate collections

In C# 3.0, the addition of Language-Integrated Query (LINQ) to the language changed forever the way collections are queried and manipulated.  Since then, if you’re using iterative statements to manipulate collections, you didn’t use LINQ when you probably should have.  

Some C# programmers don’t even know of LINQ’s existence, but fortunately that number is becoming increasingly small. Many still think, though, that because of the similarity between LINQ keywords and SQL statements, its only use is in code that queries databases.

While database querying is a very prevalent use of LINQ statements, they actually work over any enumerable collection (i.e., any object that implements the IEnumerable interface).  So for example, if you had an array of Accounts, instead of writing:

  decimal total = 0;
  foreach (Account account in myAccounts) {
    if (account.Status == "active") {
      total += account.Balance;
    }
  }

you could just write:

  decimal total = (from account in myAccounts
                   where account.Status == "active"
                   select account.Balance).Sum();

While this is a pretty simple example, there are cases where a single LINQ statement can easily replace dozens of statements in an iterative loop (or nested loops) in your code.  And less code general means less opportunities for bugs to be introduced. Keep in mind, however, there may be a trade-off in terms of performance. In performance-critical scenarios, especially where your iterative code is able to make assumptions about your collection that LINQ cannot, be sure to do a performance comparison between the two methods.

Common Mistake #5: Failing to consider the underlying objects in a LINQ statement

LINQ is great for abstracting the task of manipulating collections, whether they are in-memory objects, database tables, or XML documents.  In a perfect world, you wouldn’t need to know what the underlying objects are. But the error here is assuming we live in a perfect world.  In fact, identical LINQ statements can return different results when executed on the exact same data, if that data happens to be in a different format.

For instance, consider the following statement:

  decimal total = (from account in myAccounts
                   where account.Status == "active"
                   select account.Balance).Sum();

What happens if one of the object’s account.Status equals “Active” (note the capital A)?  Well, if myAccounts was a DbSet object (that was set up with the default case-insensitive configuration), the where expression would still match that element.  However, if myAccounts was in an in-memory array, it would not match, and would therefore yield a different result for total.

But wait a minute.  When we talked about string comparison earlier, we saw that the == operator performed an ordinal comparison of strings. So why in this case is the == operator performing a case-insensitive comparison?

The answer is that when the underlying objects in a LINQ statement are references to SQL table data (as is the case with the Entity Framework DbSet object in this example), the statement is converted into a T-SQL statement. Operators then follow T-SQL rules, not C# rules, so the comparison in the above case ends up being case insensitive.

In general, even though LINQ is a helpful and consistent way to query collections of objects, in reality you still need to know whether or not your statement will be translated to something other than C# under the hood to ensure that the behavior of your code will be as expected at runtime.

Common Mistake #6: Getting confused or faked out by extension methods

As mentioned earlier, LINQ statements work on any object that implements IEnumerable. For example, the following simple function will add up the balances on any collection of accounts:

  public decimal SumAccounts(IEnumerable<Account> myAccounts) {
      return myAccounts.Sum(a => a.Balance);
  }

In the above code, the type of the myAccounts parameter is declared as IEnumerable<Account>.  Since myAccounts references a Sum method (C# uses the familiar “dot notation” to reference a method on a class or interface), we’d expect to see a method called Sum() on the definition of the IEnumerable<T> interface.  However, the definition of IEnumerable<T>, makes no reference to any Sum method and simply looks like this:

  public interface IEnumerable<out T> : IEnumerable {
      IEnumerator<T> GetEnumerator();
  }

So where is the Sum() method defined? C# is strongly typed, so if the reference to the Sum method was invalid, the C# compiler would certainly flag it as an error.  We therefore know that it must exist, but where?  Moreover, where are the definitions of all the other methods that LINQ provides for querying or aggregating these collections?

The answer is that Sum() is not a method defined on the IEnumerable interface. Rather, it is a static method (called an “extension method”) that is defined on the System.Linq.Enumerable class:

  namespace System.Linq {
    public static class Enumerable {
      ...
      // the reference here to “this IEnumerable<TSource> source” is
      // the magic sauce that provides access to the extension method Sum
      public static decimal Sum<TSource>(this IEnumerable<TSource> source,
                                         Func<TSource, decimal> selector);
      ...
    }
  }

So what makes an extension method different from any other static method and what enables us to access it in other classes?

The distinguishing characteristic of an extension method is the this modifier on its first parameter. This is the “magic” that identifies it to the compiler as an extension method. The type of the parameter it modifies (in this case IEnumerable<TSource>) denotes the class or interface which will then appear to implement this method.

(As a side point, there’s nothing magical about the similarity between the name of the IEnumerable interface and the name of the Enumerable class on which the extension method is defined. This similarity is just an arbitrary stylistic choice.)

With this understanding, we can also see that the sumAccounts function we introduced above could instead have been implemented as follows:

  public decimal SumAccounts(IEnumerable<Account> myAccounts) {
      return Enumerable.Sum(myAccounts, a => a.Balance);
  }

The fact that we could have implemented it this way instead raises the question of why have extension methods at all?  Extension methods are essentially a convenience of the C# language that enables you to “add” methods to existing types without creating a new derived type, recompiling, or otherwise modifying the original type.

Extension methods are brought into scope by including a using [namespace]; statement at the top of the file. You need to know which namespace includes the extension methods you’re looking for, but that’s pretty easy to determine once you know what it is you’re searching for.

When the C# compiler encounters a method call on an instance of an object, and doesn’t find that method defined on the referenced object class, it then looks at all extension methods that are within scope to try to find one which matches the required method signature and class. If it finds one, it will pass the instance reference as the first argument to that extension method, then the rest of the  arguments, if any, will be passed as subsequent arguments to the extension method.  (If the C# compiler doesn’t find any corresponding extension method within scope, it will throw an error.)

Extension methods are an example of “syntactic sugar” on the part of the C# compiler, which allows us to write code that is (usually) clearer and more maintainable.  Clearer, that is, if you’re aware of their usage. Otherwise, it can be a bit confusing, especially at first.

While there certainly are advantages to using extension methods, they can cause headaches and wasted time for those developers who aren’t aware of them or don’t properly understand them. This is especially true when looking at code samples online, or at any other pre-written code. When such code  produces compiler errors (because it invokes methods that clearly aren’t defined on the classes they’re invoked on), the tendency is to think the code applies to a different version of the library, or to a different library altogether. A lot of time can be spent searching for a new version, or phantom “missing library”, that doesn’t exist.

Even developers who are familiar with extension methods still get caught occasionally, when there is a method with the same name on the object, but its method signature differs in a subtle way from that of the extension method. A lot of time can be wasted looking for a typo or error that just isn’t there.

Use of extension methods in C# libraries is becoming increasingly prevalent.  In addition to LINQ, the Unity Application Block and the Web API framework are examples of two heavily-used modern libraries by Microsoft which make use of extension methods as well, and there are many others. The more modern the framework, the more likely it is that it will incorporate extension methods.

Of course, you can write your own extension methods as well. Realize, however, that while extension methods appear to get invoked just like regular instance methods, this is really just an illusion.  In particular, your extension methods can’t reference private or protected members of the class they’re extending and therefore cannot serve as a complete replacement for more traditional class inheritance.

Common Mistake #7: Using the wrong type of collection for the task at hand

C# provides a large variety of collection objects, with the following being only a partial list:
Array, ArrayList, BitArray, BitVector32, Dictionary<K,V>, HashTable, HybridDictionary, List<T>, NameValueCollection, OrderedDictionary, Queue, Queue<T>, SortedList, Stack, Stack<T>, StringCollection, StringDictionary.

While there can be cases where too many choices is as bad as not enough choices, that isn’t the case with collection objects. The number of options available can definitely work to your advantage.  Take a little extra time upfront to research and choose the optimal collection type for your purpose.  It will likely result in better performance and less room for error.

If there’s a collection type specifically targeted at the type of element you have (such as string or bit) lean toward using that one first. The implementation is generally more efficient when it’s targeted to a specific type of element.

To take advantage of the type safety of C#, you should usually prefer a generic interface over a non-generic one. The elements of a generic interface are of the type you specify when you declare your object, whereas the elements of non-generic interfaces are of type object. When using a non-generic interface, the C# compiler can’t type-check your code. Also, when dealing with collections of primitive value types, using a non-generic collection will result in repeated boxing/unboxing of those types, which can result in a significant negative performance impact when compared to a generic collection of the appropriate type.

Another common pitfall is to write your own collection object. That isn’t to say it’s never appropriate, but with as comprehensive a selection as the one .NET offers, you can probably save a lot of time by using or extending one that already exists, rather than reinventing the wheel.  In particular, the C5 Generic Collection Library for C# and CLI offers a wide array of additional collections “out of the box”, such as persistent tree data structures, heap based priority queues, hash indexed array lists, linked lists, and much more.

Common Mistake #8: Neglecting to free resources

The CLR environment employs a garbage collector, so you don’t need to explicitly free the memory created for any object. In fact, you can’t. There’s no equivalent of the C++ delete operator or the free() function in C . But that doesn’t mean that you can just forget about all objects after you’re done using them. Many types of objects encapsulate some other type of system resource (e.g., a disk file, database connection, network socket, etc.).  Leaving these resources open can quickly deplete the total number of system resources, degrading performance and ultimately leading to program faults.

While a destructor method can be defined on any C# class, the problem with destructors (also called finalizers in C#) is that you can’t know for sure when they will be called. They are called by the garbage collector (on a separate thread, which can cause additional complications) at an indeterminate time in the future. Trying to get around these limitations by forcing garbage collection with GC.Collect() is not a good practice, as that will block the thread for an unknown amount of time while it collects all objects eligible for collection.

This is not to say there are no good uses for finalizers, but freeing resources in a deterministic way isn’t one of them. Rather, when you’re operating on a file, network or database connection, you want to explicitly free the underlying resource as soon as you are done with it.

Resource leaks are a concern in almost any environment. However, C# provides a mechanism that is robust and simple to use which, if utilized, can make leaks a much rarer occurrence. The .NET framework defines the IDisposable interface, which consists solely of the Dispose() method. Any object which implements IDisposable expects to have that method called whenever the consumer of the object is finished manipulating it. This results in explicit, deterministic freeing of resources.

If you are creating and disposing of an object within the context of a single code block, it is basically inexcusable to forget to call Dispose(), because C# provides a using statement that will ensure Dispose() gets called no matter how the code block is exited (whether it be an exception, a return statement, or simply the closing of the block). And yes, that’s the same using statement mentioned previously that is used to include namespaces at the top of your file. It has a second, completely unrelated purpose, which many C# developers are unaware of; namely, to ensure that Dispose() gets called on an object when the code block is exited:

  using (FileStream myFile = File.OpenRead("foo.txt")) {
    myFile.Read(buffer, 0, 100);
  }

By creating a using block in the above example, you know for sure that myFile.Dispose() will be called as soon as you’re done with the file, whether or not Read() throws an exception.

Common Mistake #9: Shying away from exceptions

C# continues its enforcement of type safety into runtime. This allows you to pinpoint errors much more quickly than in languages such as C++, where faulty type conversions can result in arbitrary values being assigned to an object’s fields. However, once again, programmers can squander this great feature of C#. They fall into this trap because C# provides two different ways of doing things, one which can throw an exception, and one which won’t. Some will shy away from the exception route, figuring that not having to write a try/catch block saves them some coding.

For example, here are two different ways to perform an explicit type cast in C#:

  // METHOD 1:
  // Throws an exception if account can't be cast to SavingsAccount
  SavingsAccount savingsAccount = (SavingsAccount)account;
  
  // METHOD 2:
  // Does NOT throw an exception if account can't be cast to
  // SavingsAccount; will just set savingsAccount to null instead
  SavingsAccount savingsAccount = account as SavingsAccount;

The most obvious error that could occur with the use of Method 2 would be a failure to check the return value. That would likely result in an eventual NullReferenceException, which could possibly surface at a much later time, making it much harder to track down the source of the problem.  In contrast, Method 1 would have immediately thrown an InvalidCastException making the source of the problem much more immediately obvious.

Moreover, even if you remember to check the return value in Method 2, what are you going to do if you find it to  be null? Is the method you’re writing an appropriate place to report an error? Is there something else you can try if that cast fails? If not, then throwing an exception is the correct thing to do, so you might as well let it happen as close to the source of the problem as possible.

Here are a couple of examples of other common pairs of methods where one throws an exception and the other does not:

  int.Parse();     // throws exception if argument can’t be parsed
  int.TryParse();  // returns a bool to denote whether parse succeeded
  
  IEnumerable.First();           // throws exception if sequence is empty
  IEnumerable.FirstOrDefault();  // returns null/default value if sequence is empty

Some programmers are so “exception adverse” that they automatically assume the method that doesn’t throw an exception is superior.  While there are certain select cases where this may be true, it is not at all correct as a generalization.

As a specific example, in a case where you have an alternative legitimate (e.g., default) action to take if an exception would have been generated, then that the non-exception approach could be a legitimate choice.  In such a case, it may indeed be better to write something like this:

  if (int.TryParse(myString, out myInt)) {
    // use myInt
  } else {
    // use default value
  }

instead of:

  try {
    myInt = int.Parse(myString);
    // use myInt
  } catch (FormatException) {
    // use default value
  }

However, it is incorrect to assume that TryParse is therefore necessarily the “better” method.  Sometimes that’s the case, sometimes it’s not. That’s why there are two ways of doing it. Use the correct one for the context you are in, remembering that exceptions can certainly be your friend as a developer.

Common Mistake #10: Allowing compiler warnings to accumulate

While this one is definitely not C# specific, it is particularly egregious in C# since it abandons the benefits of the strict type checking offered by the C# compiler.

Warnings are generated for a reason.  While all C# compiler errors signify a defect in your code, many warnings do as well. What differentiates the two is that, in the case of a warning, the compiler has no problem emitting the instructions your code represents. Even so, it finds your code a little bit fishy, and there is a reasonable likelihood that your code doesn’t accurately reflect your intent.

A common simple example is when you modify your algorithm to eliminate the use of a variable you were using, but you forget to remove the variable declaration. The program will run perfectly, but the compiler will flag the useless variable declaration. The fact that the program runs perfectly causes programmers to neglect to fix the cause of the warning. Furthermore, programmers take advantage of a Visual Studio feature which makes it easy for them to hide the warnings in the “Error List” window so they can focus only on the errors. It doesn’t take long until there are dozens of warnings, all of them blissfully ignored (or even worse, hidden).

But if you ignore this type of warning, sooner or later, something like this may very well find its way into your code:

  class Account {
  
      int myId;
      int Id;   // compiler warned you about this, but you didn’t listen!
  
      // Constructor
      Account(int id) {
          this.myId = Id;     // OOPS!
      }
  
  }

And at the speed Intellisense allows us to write code, this error isn’t as improbable as it looks.

You now have a serious error in your program (although the compiler has only flagged it as a warning, for the reasons already explained), and depending on how complex your program is, you could waste a lot of time tracking this one down. Had you paid attention to this warning in the first place, you would have avoided this problem with a simple five-second fix.

Remember, the C# compiler gives you a lot of useful information about the robustness of your code… if you’re listening. Don’t ignore warnings. They usually only take a few seconds to fix, and fixing new ones when they happen can save you hours. Train yourself to expect the Visual Studio “Error List” window to display “0 Errors, 0 Warnings”, so that any warnings at all make you uncomfortable enough to address them immediately.

Of course, there are exceptions to every rule.  Accordingly, there may be times when your code will look a bit fishy to the compiler, even though it is exactly how you intended it to be. In those very rare cases, use #pragma warning disable [warning id] around only the code that triggers the warning, and only for the warning ID that it triggers. This will suppress that warning, and that warning only, so that you can still stay alert for new ones.

Wrap-up

C# is a powerful and flexible language with many mechanisms and paradigms that can greatly improve productivity.  As with any software tool or language, though, having a limited understanding or appreciation of its capabilities can sometimes be more of an impediment than a benefit, leaving one in the proverbial state of “knowing enough to be dangerous”.

Familiarizing oneself with the key nuances of C#, such as (but by no means limited to) the issues raised in this article, will help optimize use of the language while avoiding some of its more common pitfalls.

Looking to hire top engineers? Check out Toptal's C# developers!
Editor's note: want posts just like this delivered straight to your inbox? Subscribe below to receive our latest engineering articles.
Subscribe to our engineering blog for the latest tips

Comments

Julio Szabo
Great post!! Congrats!!
Soonts
“Using iterative (instead of declarative) statements to manipulate collections” — questionable. LINQ statements should only be used for simple processing. Complex LINQ statement is write-only code: hard to read and understand, very hard to debug, and thus expensive to maintain. “Using the wrong type of collection for the task at hand” — this is indeed very common mistake. However, the article gives no clue how to choose the correct collection. For small collections (a dozen of items) choose whichever is easier to use, for large collections however (thousands and more) you should carefully consider whether the collection should be ordered or no, and which operations will prevail, only then choose based on the big O notation of that operation.
R. K. Ich
Your second point about collections is well received, but your first point I have to disagree with: "...hard to read and understand": LINQ has enough ubiquity and history to render this complaint pointless. I saw the LINQ statement first and I totally got what it was doing immediately. It comes down to well-defined variables/functions. "very hard to debug": Same thing applies from aforementioned statement. Maybe a first year .NET developer might have some difficulty, but descriptors like "very hard" are highly subjective. I find LINQ very easy to debug.
R. K. Ich
Kudos! Good article.
Soonts
“I saw the LINQ statement first and I totally got what it was doing immediately” — it means you saw simple (by my standards) LINQ statements, in the cases where the decision to use LINQ was wise. Which is not always the case. “descriptors like very hard are highly subjective”. Fine, let’s lets about objective things. You cannot step in/step over, which becomes problem e.g. if the LINQ contains non-MS extension methods — you can’t just step in to find out what it does. You cannot use intermediate IDE window with statements that have lambdas. Unlike imperative code, you can’t review intermediate results as they’re not kept in variables but lost in the call stack. Intermediate data is very often implicitly typed and unnamed. This list goes on and on… P.S. I saw a funny article entitled “What if Visual Studio had Achievements?”. The relevant item is #2 that says “Job Security – Written a LINQ query with over 30 lines of code”. In my previous comment, I was talking about _that_ kind of LINQ.
Uchendu Nwachuku
This is a surprisingly good article for a top-10 list. I'm definitely going to adjust my coding style based on it.
R. K. Ich
Sure it was a simple example. But please realize anyone can obfuscate or complicate any code, LINQ being no exception. I do concur with you about the limitations of debugging in LINQ, but this does not ipso facto warrant a dismissal of the technology - there's always a less than optimal use of any technology. Context determines everything.
Павел Масюк
My personal favourite is #9. Never use catching as one of an intended paths in a workflow, do checks. Catching exceptions is a more complex procces then you may think.
hallo@mail.com
While I agree with the caption of #9, I disagree about the message it sends. If there is a valid execution path for all possible cases, you should NOT use exceptions. In a end user application, there should be checks to ensure the valid state of the application. A invalid CastException or similar should not happen. This makes it possible to reserve Exception only to real errors and not a valid program use case.
Kevin Dondrea
Thank you. I loved this article.
Serge Zinin
I can't stop watching this animation :D
Leo Muller
Can you put a breakpoint in Linq? For example, if you have above statement (account.Status == "active") , and want to check why one is missing (because the status is "Active") Secondly, what about performance? Doesn't Linq do the same iteration internally? The Linq looks very elegant and convincing though, but can it sometimes be misleading? Giving people to think that this is an actual SQL statement?
Ivan Fraixedes
Nice post. Just let you know that in code snipped of "Mistake #3", the sting comparison instruction "Console.WriteLine(s.Equals("Straße", StringComparison.CurrentCulture)); " appears in two times, one in the "output false" section and also in "output true"
Paul
I had to look at this twice, but it's Straße and straße
Ivan Fraixedes
Ok, cool. I took a look several times and I couldn't see it, and I used the browser search to make sure, and it found me the two lines, so it searches "case insensitive" :S Sorry mate, for the inconveniences, I think that I'm not having a good Monday.
bert michielsen
Yes , you can put a breakpoint within a LINQ clause, provided it is multiline, but you can always make it multiline.
Alejandro Varela
since the compiler don't allow us to compile code like "Point p = null;" (because p is a valuetype)... why can happily compile code as " if (p == null) ... ; " ?
anon ymous
c# automatically casts p to a Nullable<Point> in the if statement. After that, comparing the Nullable Point to null is perfectly valid. I did expect that it would give a warning because it doesn't really make sense to do that...
Techy
My take from this list is wow, serious language design issues! structs passed by value, who thought it a smart idea to repurpose a well known keyword into something entirely different with many repercussions? LINQ is its own special little area of hell IMNSHO. Let's use an out of language tool to simulate a DB query on a collection and treat variables with a different ruleset. Sounds smart to me, no problems I can foresee. <-- sarcasm… And lastly, to be able to assign an object as another and fail, with no exception? That's just asking for problems. Fortunately, my coding practices don't include any of these aberrations when I coded C#, but still, wow.
Mező Gábor
You have missed my favorite: var user = new User(); user = db.GemmeAUser(userId);
David Mason
There are some good points in this article, but I disagree with the exception handling. Using "as" and "TryParse" should always be preferred when casting types, then the null's handled accordingly. The main reason for this is performance. Generating an exception is a very heavy operation, and whilst they should always be planned for and handled, if you can get away without using them, you should do so.
Baimangal
Common Mistake #0. Getting involved with C sharp.NET at all.
brianm101
and Common Mistake #11 is using such a badly designed language in the first place!
brianm101
In our engineering section use of LINQ is rejected in code reviews, its viewed as unnecessary and just complicates code. Same applies to other unnecessary language additions. Making languages more complicated and code more terse to save a few keystrokes is plain stupid. Not just C# see the same thing in C++ and its slowly getting worse
Lanka van Dort
Great article! Thanks so much for sharing..
Xenon10
Our engineers reject anything that's not assembly code because it is too terse and not optimized manually.
Xenon10
Because Java guys cannot C#. But I don't really care I think Python is awesome, I am writing right now a mobile OS in it.
Xenon10
Yea, totally agree, properly designed ones would be called when the object won't have any other references to it. Wait, isn't this the what the finalizer does?
Xenon10
Exceptions for casting instead of checking for null is wrong. You got this one reversed. Applying your logic you shouldn't check anything, you just wrap everything in a giant try/catch all
thx1200
I don't know if you realize, but this website is slow as molasses on FireFox. Not sure who is at fault, the site developer or Mozilla, but the user experience is terrible.
Johan Pingree
Based on #3 shouldn't the code example in #4 look like: account.Status.Equals("active", StringComparison.CurrentCultureIgnoreCase) :-)
tmcgill
Repurpose? Yes, structs in C# are rather different than in C or C++, but the fact that they are passed by value is not one of those differences. And you can't simply assign an object as another and fail without exception (such as "myCat = myDog;"), you have to explicitly use the "as" keyword to do that ("myCat = myDog as Cat;"), something you'd only do in circumstances where null on failure to convert is the behavior you want, something common enough that it is a nice syntactical shortcut.
Johan Pingree
Silly me, put the pic in the wrong place. Hopefully I didn't annoy anyone!
Patrick Ryder
This false dichotomy is exactly what leads too many developers to make this mistake. There is a third option: Let the exception propagate up the stack to an exception handler placed in an appropriate spot, where it can respond in a meaningful way.
Patrick Ryder
It's the "handled accordingly" part that's not always so straightforward. Sometimes the context you need to handle it accordingly is only available several frames up the stack. This is why exceptions were invented, to unwind the stack to that point. There is no reason to purposefully avoid using a language feature in a situation it was designed for.
Patrick Ryder
I don't think we disagree here. #9 was specifically intended to remind the programmer that the condition you started your sentence with ("_if_ there is a valid execution path...") does not always hold. The point is to determine whether or not that condition holds, and choose the appropriate method, as opposed to automatically choosing the one that doesn't throw the exception.
brianm101
Actually not quite the same, and of course it doesn't have the ease of use of a destructor, so you can't easily use any of common design patterns to close resources like file handles.
brianm101
Written in c#. ? :)
lassevk
Yet you dismissed non-LINQ code just as easily. I think the point here is that the guideline should not be "write every query/loop as a LINQ if possible".
Igor
You can put a breakpoint in any place, including lambdas - with right-click menu.
Torrey Brown
Great read. Loved the article!
Alejandro Varela
i agree with you, it doesn't make sense. thank you!
Garrett Alain Colas
I wish I could show you some of the LINQ I deal with at my work. 30+ lines of code, all within a methods return statement, utilizing 3-4 ternary operators, also it's a double level LINQ call. I love LINQ to interact with SQL in C#, but When it comes to arrays or lists, It's very rare it is more readable or more maintainable than a classic for loop. I decided to put a snippet on pastebin for you: http://pastebin.com/0AauZVit The above code is what LINQ always ends up looking like, at best.
Mihail Slavchev
Correct me if I am wrong but I think Common Mistake #2 is not actually an issue. C# compiler will not compile a code that compares anything of value type with null (see error CS0019 at MSDN documentation).
Mike Cattle
The difficulty is that if you're using LINQ-to-Entities, for example, the "account.Status.Equals("active", StringComparison.CurrentCultureIgnoreCase)" doesn't get translated into a SQL statement, and will generate a compiler error, on account of String.Equals(String, StringComparison) being a .NET Framework method, not a T-SQL method. (While perhaps future versions of EF may support the translation, I wouldn't count on it, particularly since T-SQL doesn't have a direct correlation for the StringComparison enumeration.) On the other hand, if you were doing an in-memory query using LINQ-to-Objects, it would work just fine.
Johan Pingree
Mike, thanks for taking the time to reply with an excellent explanation!
brianm101
We only allow our interns to use assembly code, the rest of us just use machine hex codes.........
Goran Obradović
Great read! I can confirm from own experience that #5, #9 and #10 are incredibly common among "senior" developers. #5 can happen to you if you don't understand how LINQ works, #9 can happen to you if you don't know what are exceptions and how it needs to be used in application (is there for reason and is not same as error), but #10 can only happen to a team where most people don't care at all about what they do. It makes me very sad when I see project with hundreds of warnings, it says a lot about general quality of code.
Leo Muller
Don't get me wrong.... 1. Nice article !! 2. LINQ can be used in many places in a good and efficient way. Just think XML or trying to sort objects in a class. 3. Regarding #1 reference vs value, this is really problematic for programmers, because there is no intuitive way to know that if you pass a class (e.g. datarow) to another function it will work differently than if you pass an integer to that function. Of course one learns this quickly ;-) And of course, C# is great, because it is so intuitive
RichardW1001
The problem with those snippets isn't Linq, the same thing as a foreach would be just as bad. The problem there is doing too much inside the object initialiser, which could just as easily do imperatively. If you were to replace the anonymous method with a more sensibly refactored equivalent, using Linq there would be fine.
ExcellentNews
Mistake #1 is using C#. The list of "advantages" of C# over C++ is bogus. C# is just as vulnerable, just as "complex" (whatever that means), and just as prone to errors (albeit of different nature) as C++. It makes a lot of things that are simple in C++ really hard (while the converse is less true). It is NOT cross-platform even within the MSFT ecosystem (and is even more hostile to cross-platform development if you consider non-Microsoft platforms). To date, C++ remains the best standard for cross-platform development. Finally, C# is much much much slower. So why is there C#? Simple - it's the Miscrosoft answer to Java. They saw Java gain traction, felt threatened, and decided to push their own "standard" to lock developers in. Java itself was an attempt to break Microsoft hold on developers in the 90s. It all has to do with mega-monopolies, and NOTHING to do with better, easier development.
ExcellentNews
Well said. See my post on top of the thread as to the real nature of C#.
Vorname Müller
C# raises productivity by 5 to 10 times - thats a fact, in conjunction with a tool like ReSharper even more. C++ gives more flexibility, especialy by excessive usage of templates. But this leads to code that is hard to 'debug' - yee need to debug the compilation process. C++ isn't generally faster than C#, thats a myth - measurements (Image processing) show a different picture: C# gives the same or sligtly lower speed, BUT with much cleaner and more maintainable code. A simple Parallel.For is nearly on the same level as TBB based parallel processing. Performance is not all, the performance crititical code is usually not more then 5...10 percent of an application. I made the best expirences with some (template flooded) native C++ code for hardware related / performance critical tasks, a managed C++ wrapper for it as a bridge and C# as the language for common application development. Instead of this senseless 'Whats the best langunage' dicussions we should follow the rule: Use always the best suitable tool for a task. And combine them as figured about ealier. This gives you both, performance AND a short time to market. Btw: Batch files/Shell scripts have a performance that is even worse compared to C++/C#, but are the glue in nearly every system...the right tool for the task.
ExcellentNews
You make some good points, esp. on having the right tool for the task. But I have to "raise an exception" with the following: "C# raises productivity by 5 to 10 times - that's a fact" "The Earth is 2 times cooler than it was 100 years ago - that's a fact" "Smoking gives you healthy lungs - that's a fact" You can "prove" each of these statements with selected data from selected industry sources. Who exactly is more productive with C#? Probably true for poorly-trained graduates of a 2-year technical program who have not been trained in C or C++. Developing a GUI for Windows? Probably true too, because MSFT has done their darnedest to push C# on developers and designed their libraries to that effect. The reason why your image processing application in C# is nearly as fast is probably because it uses an image processing library written in C :) In general, more flexibility equates more productivity. Yes, it gives you more options to screw things up, but the answer to that is better training, not less flexibility. C# takes away a lot of useful tools that C++ offers to an experience developer, but the WORST is that it locks you on MSFT platforms. And don't even get me started on Java...
g3rmain
What's wrong ?
kennymac
brianm101 I don't agree, C# is a garbage collected language, so it's much simpler, destructors aren't required because of reference counting! There are a fe subtleties for long lifetime, shared, unmanaged resources, but by eliminating the need for manual memory management, you remove an entire class of bugs from C# applications. Check out http://blogs.msdn.com/b/tom/archive/2008/04/25/understanding-when-to-use-a-finalizer-in-your-net-class.aspx More on garbage collection http://msdn.microsoft.com/en-us/library/ms973837.aspx Xenon10, finalizers do look like C++ destructors, but their role is very different, they run late and on a single finalizer thread; they will block the whole world until their processing is complete. Suggest you check out Richter's book CLR via C# http://amzn.to/1vli0Vb. If you have a shared resource whose ownership isn't clear, falling back on a finalizer is a quick but v slow fix. Sure, when all references go out of scope, it will be released, and finalize will be called - - but declaring a finalizer means the object won't be garbage collected - - who knows when its resources will be freed. Use iDisposable, keep unmanaged resources open for only a short time.
brianm101
Kennymac: Having a true destructor that is actually called when the object goes out of scope or deliberately destroyed, is a very powerfully and usefull feature. You then know exactly when resources are going to be released. For example a class that uses a large amount of memory or a file handle, all can be closed off in the destructor, and you know when it will occur! I use both c++ and c#, c++ wins hands down. Although a backup garbage collector would be useful, but there are smart pointers for that now! Lack of a determinist destructor was one of the main criticisms of c# when it was being designed - Microsoft arrogance stopped c# being a better language than it is.......
kennymac
@brianm101:disqus They are both fantastic languages, but are based on v different memory paradigms. On performance of garbage collected languages, check out this SE Radio discussion with Martin Thompson (referring to Java mainly, but everything equally applicable to C#). http://www.se-radio.net/2014/02/episode-201-martin-thompson-on-mechanical-sympathy/
Volker Roth
RichardW1001 is right. Let one method do one thing. Return e.g. IEnumerable from that method and proceed. So it's perfectly testable as well. There is no problem with LINQ. For me it makes code MORE readable.
Volker Roth
So the fact is, no one at your compny understands LINQ?
Volker Roth
LINQ to the rescue: Some should quickly read some stuff about functional programming in C#.
Matthew Blott
All very interesting, I was pleased to find a few of these I already watch out for. The first one did throw me though, although to be fair to myself (and anyone else reading this) there was no indication of what we were looking at (i.e. a struct). And then it suddenly occurred to me - and I'd be very interested in some feedback on this - I have never in the over ten years I have been programming with C# encountered another developer, software vendor or third party project that uses structs. Never. Ever.
mony1
<a href="http://www.nsmat-dammam.org/%D8%AE%D8%AF%D9%85%D8%A7%D8%AA-%D8%A7%D9%84%D8%AA%D8%B3%D9%84%D9%8A%D9%83-%D9%88%D8%A7%D9%84%D8%AA%D8%B3%D8%B1%D8%A8%D8%A7%D8%AA/%D8%B4%D8%B1%D9%83%D8%A9-%D8%AA%D8%B3%D9%84%D9%8A%D9%83-%D9%85%D8%AC%D8%A7%D8%B1%D9%8A-%D8%A8%D8%A7%D9%84%D8%AF%D9%85%D8%A7%D9%85/">تسليك مجاري بالدمام</a>
Moises Alexander Salazar Vila
Unnecessary Instance at the first line, it should be: var user = db.GemmeAUser(userId);
Shivaji Varma
Hi, In #3 sample is wrong. The command 'Console.WriteLine(s.Equals("Straße", StringComparison.CurrentCulture));' is kept in both 'Flase' and 'True' scenarios.
Syntax
There are many error programmers make which is a disadvantage to them, but the main one is they stop learning new things. http://penlr.com/programming/errors-programmer-cant-blame-compiler/
ling maaki
More about ...<a href="http://csharp.net-informations.com/collection/list.htm">C# List</a> Ling
Hans Zassenhaus
Common Mistake #9 is not a very good explanation. The If...then construct shown is really a business rule and not and exception, per se. the Try...Catch construct should, IMHO, be reserved for unexpected error/exception conditions and not used for exceptions that in fact define a business rule. The use of a default value, IMHO, is a business rule implementation.
comments powered by Disqus
Subscribe
Subscribe to Engineering Articles by Email
Trending articles
Relevant technologies