C# Corner

5 C# Traps to Avoid

C# Corner columnist Patrick Steele offers a heads up on five gotchas that can trip up even veteran C# programmers.

Make no mistake, C# is an impressive and powerful programming language that serves both hobbyist and enterprise developers alike. But a tool as powerful as C# doesn't come without its sharp edges. In this installment of C# Corner, I want to look at the five most common and most dangerous traps that developers will want to avoid when programming in C#. Some of these gotchas have been around since C# 1.0 (and still manage catch developers) while others are native to newer versions of C#, including C# 4.

Trap #1: Mistaking "var" as Late-bound
C# 3.0 introduced the "var" keyword as a way to let the compiler infer the type of variable based on the expression used to initialize the variable. Here's a simple example:

	var counter = 15;

In this example, the C# compiler will create an int variable called counter and initialize it to 15. Where var really shines is in situations like this (C# 2.0):

	IEnumerable<Customer> customers = GetCustomers();

With var, we can simplify this to:

	var customers = GetCustomers();

The C# compiler sees that GetCustomers returns an IEnumerable<Customer> so it defines "customers" as that type. The "var" keyword also removes some of the redundancy we used to have:

	List<String> names = new List<string>();

If the assignment is creating a List<String>, why do we need to define the variable as a List<String>? The var keyword infers this from the expression:

	var names = new List<string>();

However, this is a <i>compiler</i> enhancement. While the C# "var" keyword exists in JavaScript, they are totally unrelated. And it has no relation to the "variant" data type from Visual Basic 6. The "var" keyword saves you some typing by inferring the type at compile time. The real type is emitted into the IL. So the following two assignments:

	int age = 29;
	var age = 29;

Produce the exact same IL when compiled. Don't fear the "var" keyword. It can save you time and typing.

Trap #2: Be careful with optional parameters
C# 4.0 introduces "optional" parameters. An optional parameter defines a default value to be used if the caller doesn't supply a value. In the past, the concept of default values for parameters was realized by overloading:

		public decimal GetTemperature()
		{
			return GetTemperature(false);
		}

		public decimal GetTemperature(bool inCelsius)
		{
		}

With optional parameters, you write one method and define a default value:

		public decimal GetTemperatue(bool inCelsius = false)
		{
		}

While functionally equivalent, there's a big difference in how they are implemented.

With overloading, a call to GetTemperature() will execute a call to GetTemperature(false). Suppose this method is in a utility library. You decide the new default should be to return temperatures in Celsius. You change the code in GetTemperature to "return GetTemperature(true);", recompile and drop it in your directory with the consuming application. Everything is good to go and temperatures are now returned in Celsius by default.

When using optional parameters, the default "false" value was baked in to the calling assembly at compile time. Consumers of your utility library won't get the new default value unless they are recompiled. Be careful when creating public API's with optional parameters!

Trap #3: Deferred Execution in LINQ
LINQ makes querying of data so much easier than for/each loops with nested if blocks and other conditional logic. But remember that all of that fancy LINQ stuff is simply syntactic sugar over method calls.

Consider this simple example of a method using LINQ-to-SQL to get a list of customers:

	public IEnumerable<Customer> GetCustomers()
	{
		using(var context = new DBContext())
		{
			return from c in context.Customers
			       where c.Balance > 2000
			       select c;
		}
	}

Seems pretty harmless -- until you get an "ObjectDisposedException" when you try and enumerate the collection. Why? Because LINQ doesn't actually perform the query until you try and enumerate the results. The DBContext class (which exposes the Customers collection) is disposed of when this call exits. Once you try and enumerate through the collection, the DBContext.Customers class is referenced and you get the exception.

A simpler example:

	decimal minimumBalance = 500;
	var customersOver500 = from c in customers
where c.Balance > minimumBalance select c;

	minimumBalance = 200;
	var customersOver200 = from c in customers
where c.Balance > minimumBalance select c;

	int count1 = customersOver500.Count();
	int count2 = customersOver200.Count();

Suppose we have four customers with the following balances: 100, 300, 400 and 600. What will count1 and count2 be? They'll both be 3. The "customersOver500" references the "minimumBalance" variable, but the value isn't obtained until the query results are iterated over (through a for/each loop, a ToList() call or even a "Count()" call as shown above). At the time the value is used to process the query, the value for minimumBalance has already changed to 200, so both LINQ queries produce identical results (customers with a balance over 200).

If you have cases where you want to ensure your LINQ queries are evaluated right away, convert them to a List via ToList(), or to an array via ToArray().

Trap #4: Overloading Methods in a base class
This is an interesting one that can trip you up if you're not aware of how .NET resolves overloads. Suppose you have the following classes:

	public class MyBase
	{
		public void DoSomething(int a)
		{
			Console.WriteLine("MyBase");
		}
	}

	public class MyDerived : MyBase
	{
		public void DoSomething(long a)
		{
			Console.WriteLine("MyDerived");
		}
	}

What gets printed with the following code:

	MyDerived d = new MyDerived();
	d.DoSomething(5);

Most people assume the DoSomething(int) overload on the base class is a better match than the DoSomething(long) overload. However, since the variable is typed as a "MyDerived", that version of DoSomething will be called. The .NET runtime always favors the most-derived compile-time type.

Can you force a call to the base class DoSomething with a cast?

	MyDerived d = new MyDerived();
	d.DoSomething((int)5);

Nope. Remember what we just said: The .NET runtime always favors the most-derived compile-time type. In this case, .NET will still call DoSomething(long). In general, you should avoid overloading methods defined in base classes.

Trap #5: Forgetting to unsubscribe from events
This has been around since the first version of C#, but still causes bugs from some people.

As you're aware, the .NET runtime employs a very smart garbage collector to clean up objects in memory that are no longer used. It determines "used" by keeping track of what objects reference other objects (which reference other objects and so on). Once an object is found to not have any other objects referencing it, it's available for garbage collection and the memory can be released.

Let's consider a simple scenario. You have a main application form. This form opens a child form to do some editing. The edits need to update some status panel on the main form. To achieve the updates, your child form exposes an event that the main form can subscribe to. You handle the creation of the child form like this:

	using(var child = new ChildForm())
	{
		child.StatusUpdated += new EventHandler(child_StatusUpdated);
		child.ShowModel(this);
	}

You've wrapped your child form in a "using" block to make sure the Dispose method is always called. That's fine -- the Dispose call will release unmanaged resources (the window handle, child controls, etc.), but your main form is still subscribed to the StatusUpdated event of your child form. That means the garbage collector can't free up the memory used by the child form. Each time this code is hit, a new child form is created and its memory will never be released until the main form is no longer referenced.

Always make sure you unsubscribe from events you've subscribed to. It allows your unused objects to be collected sooner and puts less pressure on the garbage collector.

Quick Word of Thanks
I'd like to thank Bill Wagner, Dennis Burton and Chris Marinos for their assistance with this article. Hopefully their able assistance will help you sidestep some of these common C# traps in the future.

Do you have a C# trap you'd like to share? Email me at patrick@mvps.org.

About the Author

Patrick Steele is a Senior .NET Developer with Billhighway in Troy, Michigan. A recognized expert on the .NET Framework, he is a Microsoft MVP award winner and a presenter at conferences and user group meetings.

comments powered by Disqus

Reader Comments:

Mon, Nov 26, 2012 Pennsylvania, US

Interesting things to consider, though I'm fairly certain the other commenters are correct about #5. I would like to add that it can be bad to use the "var" keyword as much as you are here. I can't say that you absolutely shouldn't, but I consider it bad practice to use "var" except in the LINQ context for which it was created. Of course, this is also a merely code style argument and does not affect the compiled code in any way.

Sat, May 7, 2011 Ofir Israel

Great Article - but #5 is wrong. There is not problem when subscribing to a child form events, as, in fact, the child form holds a reference to the parent form event handler. The problem is when the child subscribe to an event on the parent form (or to a static event for example). Thanks Again. Ofir.

Tue, Apr 5, 2011

#5 is WRONG. Child holds a reference to the parent (via delegate) and it does NOT prevent the child from being collected by garbage collector

Fri, Feb 18, 2011

I don't agree with #2 What if the consumer is calling both GetTemperature() and GetTemperature(true) If you change the base as you say. Both of the result will be the same(Celsius).

Fri, Feb 18, 2011 JoeNet SLC

#4 is called Versioning. What you are showing is Hiding a base class member. The C# language is designed such that versioning between base and derived classes in different libraries can evolve and maintain backwards compatibility. This means, for example, that the introduction of a new member in a base class with the same name as a member in a derived class is not an error

Thu, Feb 17, 2011 dev01 Seattle

#4. Isn't the reason for this behavior that the base class method lacks virtual keyword.

Thu, Feb 17, 2011 tridy Sweden

I don't really agree with the VAR thing. IMHO, for the readability either left or right side needs to specify the type. this looks ok: var employeeList = new List; does not look good: var employeeList = GetEmployees(); var i = 5 looks weird. With var you can speicify int but not long but you can write 5.0. I still like explicit declarations in this case. Jut my 5 cents.

Sat, Dec 4, 2010

#5 is certainly wrong. The message should be: Unsubscribe from events provided by long-living instances; otherwise you have a memory leak (an bad runtime performance, as well).

Tue, Nov 9, 2010 David Ching San Francisco Bay Area, CA, USA

Thankfully, #5 is incorrect as DAVID commented (SEP 2, 2010). This article (http://msdn.microsoft.com/en-us/library/ms366768.aspx) says you should unsubscribe from events before you dispose of a subscriber object. As long as the publishing object holds that reference, garbage collection will not delete your subscriber object." Here, "child" in this article's example is the publisher.

Wed, Oct 20, 2010 CDull

I was also a bit confused by this, and I'm pretty sure David is correct, and the article is wrong. The child has a reference to the parent in the invocation list. Child goes out of scope, this ref can certainly be collected. I appreciate tip #2, though. Haven't gone to c#4 yet, but definitely something to keep in mind when I do

Wed, Oct 6, 2010 Pankaj Nikam India

The most interesting part of the article was about events and the overloading in base classes. It was an eye opener although I dont do such things but it certainly made me aware about many things. Thanks a lot for such brilliant article :)

Wed, Oct 6, 2010 Jeff

I wouldn't call #1 a "trap", since I wouldn't fault anyone for being explicit and more readable. In fact I'd encourage it. 2,3 and 4 are excellent tips. 5 I'm not so sure about....

Tue, Oct 5, 2010 Joseph Ackerman

>>Don't fear the "var" keyword. It can save you time and typing.<< Yes, it cans save some typing but you are sacrificing readability (and ease of maintenance) by making it harder for another programmer to understand your code. The typing time that you save will be spent two years from now when some other poor schmoe has to look up your function to figure out what kind of data it returns.

Tue, Oct 5, 2010 alvaradojl Honduras

great article, very neat examples!!!

Tue, Sep 21, 2010

MyBase mb=new MyBase(); mb.DoSomething(a); MyDerived md=new MyDerived(); ((mb)md).DoSomething(a);

Tue, Sep 21, 2010 Ganesan Krishnamurthy Bangalore

Though "var" is still getting resolved at compilation time, it should not be considered equivalent to use "specific" variable type, as "var" tends to be bit slower in performance because of time taken to resolve this type,,,

Wed, Sep 15, 2010 Fred

It is possible to explicitly call the base method:

int a=5;
long b = 5
//calls the base
MyBase mb=new MyDerived();
mb.DoSomething(a);
//also calls the base
MyDerived md=new MyDerived();
((MyBase)md).DoSomething(a);

//next line doesnot compile because b is a long
//((MyBase)md).DoSomething(b);

Mon, Sep 13, 2010

Pretty Goood and well explained.

Fri, Sep 10, 2010 Srinivasan Hyderabad,INDIA

Superb Article,I feel there are more traps in C# that a programmer must avoid.So please do publish more articles regarding the same.Also VS2010 introduces parallel programming so if you can publish traps in C# w.r.t parallel programming that would be very much useful

Fri, Sep 3, 2010 GA

I have to agree with the first comment above made by David regarding #5. I don't believe #5 is correct.

Thu, Sep 2, 2010 David

Are you sure about #5? I think it's the opposite. Since the delegate object behind the event belongs to the child form, it will be garbage collected when the child form is collected. If, on the other hand, the child form would subscribe to an event of the main form -- then we'll get a memory leak, because the long-lived form keeps a reference to the short-lived form. The general rule is to be careful when a short lived object subscribes to a long lived object (e.g. a static object).

Wed, Sep 1, 2010 Ross Carlson Colchester, VT

David, the article talks about overloading, not overriding. Excellent article.

Wed, Sep 1, 2010 David V Laguna Niguel, CA

Nice article. I would disagree with one conclusion, though. " In general, you should avoid overloading methods defined in base classes." The behavior cited is exactly the behavior I want. When I override a base class method, I don't want the base method invoked, unless I call it in my derived class method. So I would change the advice to "Don't override a base class method unless you intend to completely hide it."

Wed, Sep 1, 2010 Herberts Ngobola

Great article, very handy,

Wed, Sep 1, 2010

Nice article. Thanks.

Tue, Aug 31, 2010

Helpful..

Add Your Comments Now:

Your Name:(optional)
Your Email:(optional)
Your Location:(optional)
Comment:
Please type the letters/numbers you see above

.NET Insight

Sign up for our newsletter.

I agree to this site's Privacy Policy.