Tell me more ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

What do you consider a bug in C#, or .NET framework that Microsoft will not fix, or hasn't fixed yet?

I'm hoping the answers to this question will give us better patterns to work from, and build more robust code.

share|improve this question

closed as not constructive by Anna Lear Dec 31 '11 at 3:05

As it currently stands, this question is not a good fit for our Q&A format. We expect answers to be supported by facts, references, or specific expertise, but this question will likely solicit debate, arguments, polling, or extended discussion. If you feel that this question can be improved and possibly reopened, see the FAQ for guidance.

16 Answers

up vote 34 down vote accepted

That you have to check if an event if null before raising it

public event EventHandler SomeEvent;

public void RaiseEvent()
{
    if (SomeEvent != null)
        SomeEvent(this, null);
}

If listeners are connected, then the event contains a list of listeners, if no listener is connected, it is null (instead of just an empty list). This behavior has existed since .NET 1.0, and to me highly illogical.

share|improve this answer
3  
EventHandler is not a list...it is a delegate...while it may contain a listing of listeners assuming it should be treated as a list assumes that list like behavior is all that it can provide – Aaron McIver Feb 5 '11 at 21:36
9  
This is C# language gotcha as VB.Net doesn't have this problem. This means that the language could have done this check. – Pratik Feb 5 '11 at 22:09
7  
Note that as a result, the code above has a race condition: between the if (SomeEvent != null) check and the SomeEvent(this, null); statement, another thread could remove the last listener... The correct version requires making a local copy of the delegate, comparing this copy with null, and calling the copy. So this oversight in the language will introduce subtle bugs! – Sjoerd Feb 5 '11 at 23:55
13  
Note also that the complaint is based on a falsehood. You are not required to check the object for null before invoking it. Rather, you are required to not invoke a null object. That's subtly different. Building a null check in is just one (easy, obvious) way to avoid invoking a null object. If you can come up with a different way to guarantee that null is never invoked, then you can remove the check. Some people, for example, add a "do nothing" event handler on initialization and never remove it. Then they don't need to do the null check. – Eric Lippert Feb 7 '11 at 18:10
5  
And yet...given the += syntax in C#, clearly an event either already points to its collection of delegates, or automatically creates that collection when the first event is added. So if we don't have to explicitly initialize the multicast delegate collection, why would we have to explicitly check whether it's empty? It would make more sense if we had to do either both, or neither. – Kyralessa Dec 30 '11 at 18:35
show 12 more comments

All references are nullable by default

Anders Hejlsberg who is considerd the "father of C#" even said this was a mistake in a Computerworld interview:

50% of the bugs that people run into today, coding with C# in our platform, and the same is true of Java for that matter, are probably null reference exceptions. If we had had a stronger type system that would allow you to say that ‘this parameter may never be null, and you compiler please check that at every call, by doing static analysis of the code’. Then we could have stamped out classes of bugs.

share|improve this answer
show 2 more comments

Out of range ints as enums.

You can cast an out-of-range enum from an int and the compiler is fine with it:

enum Colour
{
    Red = 1,
    Green = 2,
    Blue = 3
}

Colour eco;
eco = (Colour)17;

See here for a longer discussion (and some replies).

share|improve this answer
2  
@bold: What's a bug from the 80's doing in language/framework that was released in 2002? – nikie Feb 6 '11 at 8:27
2  
@bold: I don't get your point. That's exactly what the compiler does when you cast from int to byte or short. Or when you downcast an object. The compiler can't check at compile time, so it emits a runtime check that throws and exception if the cast is illegal. – nikie Feb 6 '11 at 11:04
2  
@bold: The situation is the same for integers. If you have a function byte f(int x) { return (byte)x; } then the compiler knows "shit" about x at compile time, too. Yet the function will throw a runtime exception when you call f(300) because 300 cannot be represented as a byte. The compiler could do exactly the same with enum types. – nikie Feb 6 '11 at 11:57
1  
@bold: Are you sure you know how compilers, bytes and computers in general work? Your last comment gave a different impression. – nikie Feb 6 '11 at 12:51
show 12 more comments

The requirement for break statements in a switch, even though fall-through is not allowed anyway.

switch (myvalue)
{
    case 1: DoSomething();
    case 2: DoSomethingElse();
}

Won't work (compile error). You have to

switch (myvalue)
{
    case 1: DoSomething();
        break;                     // AARGH!

    case 2: DoSomethingElse();
        break;             
}

Mysteriously, however, this works:

switch (myvalue)
{
    case 1: return something;
    case 2: return somethingElse;
}

If you try to put the break statements in after the return statements, you will get an "Unreachable code detected." compile error.

share|improve this answer
15  
This is mysterious to you because you believe a falsehood. The language nowhere requires "break". The language requires that the end point of a switch section not be reachable, so that it is clear from the code that there will be no fall-through. break, continue, goto, throw, return or an infinite loop will all do, since none of them have a reachable end. – Eric Lippert Feb 7 '11 at 17:47
3  
@Eric: That objective could have been accomplished with scoping rules, i.e. one statement per case. The break statements are there because that is what the c and c++ developers were used to. – Robert Harvey Feb 7 '11 at 18:00
7  
You are correct. There are many ways to accomplish the goal of avoiding unwanted fall-through, a common source of bugs that plagues C developers. Allowing exactly one statement per switch section would achieve the goal of avoiding the unwanted fall-through, but might make it more confusing for C programmers reading the code. The designers of C# chose a rule that makes it easier for C developers to be productive in C#. There are of course many conflicting goals in any design process; the art of design is coming up with reasonable compromises in the face of those conflicting goals. – Eric Lippert Feb 7 '11 at 18:06
7  
@Eric, if the language forbids fall-through, why should the programmer have to specify not to do it? Instead of throwing an error, couldnt it just automatically jump to the end of the switch? Or is this solely to ensure that (less experienced) programmers are aware that there is no fallthrough, and are not expecting behavior that will not be.... – AviD May 4 '11 at 11:04
show 1 more comment

Closures

It may not look like it but the following code will print 10 on every line...

// In Example3a.cs
static void Main()
{
    // First build a list of actions
    List<Action> actions = new List<Action>();
    for (int counter = 0; counter < 10; counter++)
    {
        actions.Add(() => Console.WriteLine(counter));
    }

    // Then execute them
    foreach (Action action in actions)
    {
        action();
    }
}

Fixed version:

// In Example3b.cs
static void Main()
{
    // First build a list of actions
    List<Action> actions = new List<Action>();
    for (int counter = 0; counter < 10; counter++)
    {
        int copy = counter;
        actions.Add(() => Console.WriteLine(copy));
    }

    // Then execute them
    foreach (Action action in actions)
    {
        action();
    }
}

To learn why this is, check out this article by John Skeet

share|improve this answer
3  
It's the same in several other languages and it actually makes sense. But yes, it can be annoying. – delnan Feb 5 '11 at 20:58
15  
It's not a mistake. If you understand how closures work, it makes sense. How do closures lose their utility? Just create a local copy of the variable. – Matt H Feb 5 '11 at 22:16
8  
This is not C# or .NET specific. It is how closures work in all languages, where variables are mutable. This is an extremely powerful feature, permitting things that are tedious or even impossible to implement without it, while you can get the behaviour you desire by adding one line. – back2dos Feb 5 '11 at 22:21
3  
Agree with the others - the bug is in the code not the language - this is exactly how closures are supposed to work... – FinnNk Feb 5 '11 at 23:34
13  
It is a mistake. It's not a mistake in how closures work. It's a mistake in how foreach works. The loop variable is declared outside the loop, and that's what leads to the closure problem. If it were declared inside, the problem would go away. See Eric Lippert's blog post for discussion of this, and of how Microsoft has considered changing this behavior in foreach, even though it'd be a breaking change: blogs.msdn.com/b/ericlippert/archive/2009/11/12/… – Kyralessa Feb 7 '11 at 3:12
show 5 more comments

This is going to be controversial :)

Garbage Collection

Now, I've been involved with this since Java first came out (I went to a "w00t" presentation about Java at Hursley Park many years ago where a chap told us all how wonderful it was, and mentioned GC. Did you know Java initially didn't even have finalisers? Crazy, everyone at that meeting told the presenters just what a stupid idea Java was because of that). And Microsoft decided that a GC was the way to go with .NET, but although they had finalisers they didn't really like it when a quite vocal group of us on the Microsoft forums told them a lack of deterministic finalisation was a mistake. They added the IDispose as a pattern shortly afterwards, and the using keyword a few years later.

The moral of the above stories is that 2 developers of new languages just didn't quite get it. They added a great cool feature where you no longer had to worry about alloc/free of memory manually, the language feature would take care of it for you and you no longer had to worry our pretty little heads about it.

Only the problem is that they forgot something. Garbage collection is great for memory management. It is useless at object management.

Most applications revolve around object lifetimes, they don't really care to much about memory except as a 'container' to hold your objects. So having GC that frees up your memory isn't going to cut it, except in those cases where your object is simple and contains nothing more than memory anyway (which, ok, is 90% of all objects - but who cares about the simple stuff when you have complex stuff to build).

So because of this, we have the situation where we not only have various features added to the language to attempt to make up for the deficiencies, but we also have to manually manage the object lifetimes as well!

In a RAII-based language, object lifetimes are easily managed. In a GC one, we have to not only implement special IDispose methods (in addition to the finaliser, and make sure they do not conflict with issues like double-deletion, etc), and 'smart pointer' classes like SafeHandle, but we also have to manually put using statements in to ensure they are called, and remember to write your exception handling code correctly so your objects won't leak. You also have to understand what the internals of your libraries are doing, or you end up with leaks such as those when not manually un-associating event handlers on a GUI.

How wonderful this 'automatic', 'no worries' memory management mechanism is!

So, remember that GC is great for memory, bad for objects, and the problem is that the language designers didn't make a distinction between the two equating objects == memory.

The fix is of course to split the distinction. Imagine if objects were treated separately, with full automatic scoping using smart pointers. The memory underlying the object can still be allocated and freed using the GC, but the destructor (I hesitate to use either finalise or dispose as they're both band-aids) can then be implemented in a RAII fashion. Thus you'd get the best of both worlds - fast, no-leak memory, and deterministic object finalisation.

share|improve this answer
1  
+1 because I think users of GC languages should have a better appreciation for RAII – Winston Ewert Dec 30 '11 at 19:25
1  
@WinstonEwert: I agree neither RAII or GC solves all the problems. You still need to know a lot about them to use them correctly (In large systems (in small toys GC has the advantage)). What I object to is referring to GC as a superior method (its just a different method). Here Chris was poking at RAII as if this flawed made it uniquely inferior to GC. I just pointed out that problem is not a real problem. – Loki Astari Dec 30 '11 at 19:52
1  
@LokiAstari, I work every day in a system that uses shared/weak ptrs, but once you add those both in it is no longer "automatic" with "no worries". I have to explain to perfectly intelligent people all the time why they accidently created a memory leak because the pointer to some base class didn't take into account that a subclass holds a pointer to some other object that forms a loop. Unless you make the lanaguage detect these automatically (expensive overhead), then users have to be veyr careful about adding relationships between objects. – Chris Pitman Dec 31 '11 at 6:00
show 13 more comments

I would consider cold-loading times so bad that it's a bug. This is what is holding .Net back from being a first class environment for client apps. For instance, consider that on a freshly booted machine, it will often take upwards of 5 seconds to launch a simple WPF app that doesn't display anything but a blank window.

In what other environment do people put up with this? Java has the same problem, that too is primarily a server framework because of this. This problem reduces .Net on the client to being confined to in-house enterprise development (where users don't really have any choice). No one in their right mind would develop client apps for the masses in this environment.

share|improve this answer
1  
"No one in their right mind would develop client apps for the masses in this environment." - Why? It's just the startup time. If the application you're writing usually runs for longer than 2 minutes, 5s cold-start time is negligible. And IMHO this is neither a bug nor a feature. – nikie Feb 5 '11 at 23:19
3  
Use NGen: see this answer: stackoverflow.com/questions/45702/… – Sjoerd Feb 5 '11 at 23:58
3  
@Robert I use a few .NET apps and work on a major one (although in a niche market) for a living. The beauty of it is that if you're using Windows, you probably use .NET apps and you might never realize it. – Anna Lear Feb 6 '11 at 0:04
1  
...I never see more than a second of initial loading time from a .Net app – Earlz Feb 6 '11 at 2:10
1  
@Robert Jeppesen - I know of many .net commercial apps besides VS. Many of them are Microsoft apps, but there are others. Examples are: Microsoft Media Center, Microsoft Expression, Outlook Business Contact Manager, Zune client software, Paint.net, Act! (2005 and later versions), TurboTax, Family Tree Maker, etc.. – Mystere Man Feb 6 '11 at 6:48
show 5 more comments

Using() may cause object initializers to leak if an exception is thrown

If an exception is thrown within a using statement then its possble that the objects may not get properly disposed of.

For a detailed explanation of this see this article. Many people have considered this to be a bug, however this is by design.

Eric Lippert's response:

The behaviour is correct and desirable. The spec is extremely clear on this point and if an implementation of C# does not have this behaviour, they have a bug.

The spec clearly states that "using(T x = y) s;" means exactly the same thing as "{ T x = y; try { s; } finally { ... code to dispose x ... }". That is, the initialization is done outside the try-protected region, precisely so that if it fails, we do NOT attempt to clean it up. That would be dangerous.

The spec also clearly states that "t = new T() { X = x };" means exactly the same thing as "T temp = new T(); temp.X = x; t = temp;". That is, the assignment to the variable happens after the object is known to be in its fully initialized state.

These two designs work together to ensure that you never dispose a partially initialized resource. If you have a situation where you need to handle disposing of partially constructed resources then you need to write the code yourself to do that; there's no way we can generate the code for you and have any confidence that it is right. Any operation on a partially initialized resource is potentially hazardous.

Based on that answer I'm considering using the try..catch..finally blocks to ensure proper cleanup of my code if an exception occurrs, as opposed to using statements.

share|improve this answer
2  
It is true that there is a pitfall, but your example code does not show that pitfall, because you are not using object initializers. Btw. Running FxCop on such code will catch it if you have fallen into this pitfall – Pete Feb 5 '11 at 20:40
6  
The only thing that would cause csEncrypt to not be disposed is if the constructor itself throws an exception, and since the object will not be constructed at all in this case, it will not need disposing (the constructor should handle disposing any allocated resources in case of an exception). The same goes for swEncrypt. – Pete Feb 5 '11 at 21:33
1  
Pete got it. The using() block is working properly. If the constructor of the object that's being wrapped in the using() statement fails, there's nothing to clean up in the first place. Exceptions thrown by the body of the using() statement result in proper cleanup. – Anna Lear Feb 6 '11 at 0:08
2  
It's very important to note that the article is talking about object initializers and the using statement. The using statement by itself does not leak! – Dean Harding Feb 6 '11 at 2:12
show 3 more comments

Allowing any object to be used as a lock

You can synclock on any object. I'm not really sure why this is the case. Maybe the ease of writing 'synclock (this)'?

There are several consequences of this decision:

  • The easiest locking code to write is subtly wrong. If you 'synclock (this)', anyone who has a reference to your object can also hold that lock. They can accidentally create deadlocks. For example if you internally run an operation asynchronously, and it needs the lock, then a user 'helpfully' holding the lock over a couple operations will prevent them from completing.

  • Locking overhead. Every object needs a place to store their locked state and so forth. If you store it in the object then you have a space overhead on every instance of the object. If you store it in a table then you have a time overhead on every locking operation to indirectly access the data.

  • Framework overhead. The ability to lock on any object creates mysteries for programmers. What happens if you try to lock on a boxed value type? Is locking on null allowed? Do two boxed value types with the same value correspond to the same lock or different locks? What about nullable values? Null nullable values? Does casting maintain lock identity? If I mutate the object I'm locking on, does it now correspond to a different lock? If I'm holding an object as a lock, is it safe to garbage collect? Dispose? Finalize? Resurrect? Can using an instance of type X as a lock introduce possible deadlocks if I'm also using it for its intended purpose?

Personally, I would have gone with a specialized lock class. If you want to hold a lock, you use an instance of the special class. This has the nice effect of hiding locks by default and naturally allowing for different types of locks (read/write, up-to-n, non-re-entrant, etc).

share|improve this answer
1  
The C# documentation discourages using synclock(this) as other people may get a lock on you. Objects are encouraged to only lock internal private members. – Loki Astari Dec 30 '11 at 19:23
2  
If you couldn't lock on every object, it would not be necessary for people to discover they shouldn't lock on 'this' via the documentation. The existence of that documentation supports my point.Locking on every object doesn't prevent you from doing things right, it allows you to do it wrong in a way that would otherwise not be allowed. – Strilanc Dec 31 '11 at 4:49

I don't think everyone will agree but I would say their entire Ajax implementation and more specifically the UpdatePanel.

The fact that the entire page renders and gets sent back to the browser and then just a small piece of it is used completely undermines the entire concept of Ajax and just going back to the server to get what you need. Seems very hacky to me.

I am aware there are other implementations but for the Microsoft one to be so inefficient I would consider that a bug.

share|improve this answer
2  
Let's just say the whole of ASP.NET (classic) :-) ASP.NET MVC is the way it should have worked all along! – Dean Harding Feb 6 '11 at 2:15
3  
-1, that's not what an UpdatePanel does. Build a page using one and then look at the traffic using Firebug or a proxy such as Fiddler. It only sends the HTML content of the UpdatePanel + some other overhead, not the entire page. Not wishing to defend ASP.NET AJAX too stridently, because I don't think it's very good, but this criticism is off target. – Carson63000 Feb 6 '11 at 6:17
1  
UpdatePanel is evil! – Stefan Rusek Feb 6 '11 at 14:19
show 1 more comment

InternalsVisibleToAttribute

This attribute allows one assembly to be able to see internal methods, properties, and members of another assembly. This is just plain poor development practice.

Though I do not know for sure why it exists, I believe they introduced it along with their own unit test framework in order to let unit tests be able to access internals in the classes that they test. This is obviously the wrong way to write tests, as tests should test the public interface of a class.

And it is hard to explain to coworkers that are not that experienced with unit tests that I am right, and MS is wrong.

share|improve this answer
2  
If there are internal methods in a class, they should also be tested to ensure that they work right. They will be accessible by other classes in the assembly and workflows through public methods may not cover them all. – Anna Lear Feb 6 '11 at 6:29
8  
@Pete There may be integration tests that test how the two classes operate with each other, but I think it's beneficial to have a set of more basic unit tests that verify individual method correctness. – Anna Lear Feb 6 '11 at 6:42
1  
What's wrong with white-box testing, in addition to black-box testing? – nikie Feb 6 '11 at 8:23
2  
Anything possible with InternalsVisibleTo can be accomplished with reflection. public, private, internal is just compiler enforced documentation, but doesn't prevent you from messing about within the entire process' address space. I think what you're looking for is code access security. – Scott Whitlock Feb 6 '11 at 22:11
1  
If it's that hard to explain to all your coworkers, then maybe Microsoft is right, and you're wrong. – Kyralessa Feb 10 '11 at 3:32
show 3 more comments

What has bugged me lots of times already is that the XmlReader doesn't expose the stream position of a given tag, although it has that information internally.

For example I have a N GB XML stream and I need to separate a header with meta information from an inner document with the content (a quite common scenario for example in BizTalk).

To do that stream based (a must when processing multiple big documents) with .NET you have to implement something close to a full blown XML parser which then does expose the stream position. And that's not the way it should be.

share|improve this answer
show 1 more comment

Interating through a null collection with foreach throws an error. IMHO, it should just continue on. Yes I understand that it's null, but it's just annoying as annoying can be, and a real gotcha for newbies.

share|improve this answer
1  
When you're interating through a collection with foreach, I fail to see any situation where you'd care about the distinction between it being null and empty. It should just treat it like it's empty. Initialising it when it's declared is fine, and that's what I do, but my point still stands. – LachlanB Jan 30 '12 at 21:43
show 2 more comments

ObjectDataSource is very difficult to extend. You have to use reflection to access private/internal members in order to do anything useful.

share|improve this answer

The fact you can't properly load a control by classname/assembly kills me every time.

And it still hasn't been fixed:

http://stackoverflow.com/questions/450431/dynamically-loading-a-usercontrol-with-loadcontrol-method-type-object

share|improve this answer

The TotalHours of a Timespan doesn't correctly show above 24hours without using Math.Truncate. Although this is not a bug its weird functionality I believe. Has caught me out a few times when I was learning.

Edit:

Example: http://stackoverflow.com/questions/3505230/format-timespan-greater-than-24-hour

share|improve this answer
show 2 more comments

Not the answer you're looking for? Browse other questions tagged or ask your own question.