384

I was recently working with a DateTime object, and wrote something like this:

DateTime dt = DateTime.Now;
dt.AddDays(1);
return dt; // still today's date! WTF?

The intellisense documentation for AddDays() says it adds a day to the date, which it doesn't - it actually returns a date with a day added to it, so you have to write it like:

DateTime dt = DateTime.Now;
dt = dt.AddDays(1);
return dt; // tomorrow's date

This one has bitten me a number of times before, so I thought it would be useful to catalog the worst C# gotchas.

15
  • 158
    return DateTime.Now.AddDays(1);
    – crashmstr
    Oct 27, 2008 at 19:38
  • 24
    AFAIK, the built in value types are all immutable, at least in that any method included with the type returns a new item rather than modifying the existing item. At least, I can't think of one off the top of my head that doesn't do this: all nice and consistent. Oct 27, 2008 at 19:39
  • 6
    Mutable value type: System.Collections.Generics.List.Enumerator :( (And yes, you can see it behaving oddly if you try hard enough.)
    – Jon Skeet
    Oct 27, 2008 at 19:48
  • 14
    The intellisense gives you all the info you need. It says it returns a DateTime object. If it just altered the one you passed in, it would be a void method.
    – John Kraft
    Oct 27, 2008 at 21:50
  • 22
    Not necessarily: StringBuilder.Append(...) returns "this" for example. That's quite common in fluent interfaces.
    – Jon Skeet
    Oct 27, 2008 at 22:49

61 Answers 61

309
private int myVar;
public int MyVar
{
    get { return MyVar; }
}

Blammo. Your app crashes with no stack trace. Happens all the time.

(Notice capital MyVar instead of lowercase myVar in the getter.)

19
  • 114
    and SO appropriate for this site :)
    – gbjbaanb
    Oct 27, 2008 at 23:18
  • 64
    I put underscores on the private member, helps a lot!
    – chakrit
    Oct 28, 2008 at 0:00
  • 61
    I use automatic properties where I can, stops this kind of problem alot ;) Dec 19, 2008 at 15:50
  • 30
    This is a GREAT reason to use prefixes for your private fields (there are others, but this is a good one): _myVar, m_myVar
    – jrista
    Jun 26, 2009 at 8:01
  • 209
    @jrista: O please NO... not m_ ... aargh the horror...
    – fretje
    Jun 26, 2009 at 8:14
257

Type.GetType

The one which I've seen bite lots of people is Type.GetType(string). They wonder why it works for types in their own assembly, and some types like System.String, but not System.Windows.Forms.Form. The answer is that it only looks in the current assembly and in mscorlib.


Anonymous methods

C# 2.0 introduced anonymous methods, leading to nasty situations like this:

using System;
using System.Threading;

class Test
{
    static void Main()
    {
        for (int i=0; i < 10; i++)
        {
            ThreadStart ts = delegate { Console.WriteLine(i); };
            new Thread(ts).Start();
        }
    }
}

What will that print out? Well, it entirely depends on the scheduling. It will print 10 numbers, but it probably won't print 0, 1, 2, 3, 4, 5, 6, 7, 8, 9 which is what you might expect. The problem is that it's the i variable which has been captured, not its value at the point of the creation of the delegate. This can be solved easily with an extra local variable of the right scope:

using System;
using System.Threading;

class Test
{
    static void Main()
    {
        for (int i=0; i < 10; i++)
        {
            int copy = i;
            ThreadStart ts = delegate { Console.WriteLine(copy); };
            new Thread(ts).Start();
        }
    }
}

Deferred execution of iterator blocks

This "poor man's unit test" doesn't pass - why not?

using System;
using System.Collections.Generic;
using System.Diagnostics;

class Test
{
    static IEnumerable<char> CapitalLetters(string input)
    {
        if (input == null)
        {
            throw new ArgumentNullException(input);
        }
        foreach (char c in input)
        {
            yield return char.ToUpper(c);
        }
    }
    
    static void Main()
    {
        // Test that null input is handled correctly
        try
        {
            CapitalLetters(null);
            Console.WriteLine("An exception should have been thrown!");
        }
        catch (ArgumentNullException)
        {
            // Expected
        }
    }
}

The answer is that the code within the source of the CapitalLetters code doesn't get executed until the iterator's MoveNext() method is first called.

I've got some other oddities on my brainteasers page.

19
  • 26
    The iterator example is devious!
    – Jimmy
    Oct 27, 2008 at 19:49
  • 9
    why not split this into 3 answer so we can vote each one up instead of all together?
    – chakrit
    Oct 28, 2008 at 0:04
  • 13
    @chakrit: In retrospect, that would probably have been a good idea, but I think it's too late now. It might also have looked like I was just trying to get more rep...
    – Jon Skeet
    Oct 28, 2008 at 6:20
  • 19
    Actually Type.GetType works if you provide the AssemblyQualifiedName. Type.GetType("System.ServiceModel.EndpointNotFoundException, System.ServiceModel, Version=3.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089");
    – chilltemp
    Oct 28, 2008 at 20:23
  • 2
    @kentaromiura: The overload resolution starts at the most derived type and works up the tree - but only looking at methods originally declared in the type it's looking at. Foo(int) overrides the base method, so isn't considered. Foo(object) is applicable, so overload resolution stops there. Odd, I know.
    – Jon Skeet
    Jul 8, 2009 at 13:24
201

The Heisenberg Watch Window

This can bite you badly if you're doing load-on-demand stuff, like this:

private MyClass _myObj;
public MyClass MyObj {
  get {
    if (_myObj == null)
      _myObj = CreateMyObj(); // some other code to create my object
    return _myObj;
  }
}

Now let's say you have some code elsewhere using this:

// blah
// blah
MyObj.DoStuff(); // Line 3
// blah

Now you want to debug your CreateMyObj() method. So you put a breakpoint on Line 3 above, with intention to step into the code. Just for good measure, you also put a breakpoint on the line above that says _myObj = CreateMyObj();, and even a breakpoint inside CreateMyObj() itself.

The code hits your breakpoint on Line 3. You step into the code. You expect to enter the conditional code, because _myObj is obviously null, right? Uh... so... why did it skip the condition and go straight to return _myObj?! You hover your mouse over _myObj... and indeed, it does have a value! How did THAT happen?!

The answer is that your IDE caused it to get a value, because you have a "watch" window open - especially the "Autos" watch window, which displays the values of all variables/properties relevant to the current or previous line of execution. When you hit your breakpoint on Line 3, the watch window decided that you would be interested to know the value of MyObj - so behind the scenes, ignoring any of your breakpoints, it went and calculated the value of MyObj for you - including the call to CreateMyObj() that sets the value of _myObj!

That's why I call this the Heisenberg Watch Window - you cannot observe the value without affecting it... :)

GOTCHA!


Edit - I feel @ChristianHayter's comment deserves inclusion in the main answer, because it looks like an effective workaround for this issue. So anytime you have a lazy-loaded property...

Decorate your property with [DebuggerBrowsable(DebuggerBrowsableState.Never)] or [DebuggerDisplay("<loaded on demand>")]. – Christian Hayter

7
  • 11
    brilliant find! you're not a programmer, you're a real debugger. Apr 28, 2010 at 16:41
  • 26
    I've run into this even hovering over the variable, not just the watch window. May 14, 2010 at 20:43
  • 35
    Decorate your property with [DebuggerBrowsable(DebuggerBrowsableState.Never)] or [DebuggerDisplay("<loaded on demand>")]. Apr 9, 2013 at 20:31
  • 4
    If you are developing a framework class and want watch window functionality without altering the runtime behavior of a lazily-constructed property, you can use a debugger type proxy to return the value if it's already been constructed, and a message that the property hasn't been constructed if that's the case. The Lazy<T> class (in particular for its Value property) is one example of where this is used. May 14, 2013 at 15:16
  • 4
    I recall someone who (for some reason I can't fathom) changed the value of the object in an overload of ToString. Every time he hovered over it the tooltip gave him a different value - he couldn't figure it out...
    – JNF
    Aug 10, 2014 at 6:42
195

Re-throwing exceptions

A gotcha that gets lots of new developers, is the re-throw exception semantics.

Lots of time I see code like the following

catch(Exception e) 
{
   // Do stuff 
   throw e; 
}

The problem is that it wipes the stack trace and makes diagnosing issues much harder, cause you can not track where the exception originated.

The correct code is either the throw statement with no args:

catch(Exception)
{
    throw;
}

Or wrapping the exception in another one, and using inner exception to get the original stack trace:

catch(Exception e) 
{
   // Do stuff 
   throw new MySpecialException(e); 
}
6
  • Very luckily, I got taught about this in my first week by someone and find it in more senior developers' code. Is: catch() { throw; } The same as the second code snippet? catch(Exception e) { throw; } only it doesn't create an Exception object and populate it?
    – StuperUser
    Jul 8, 2009 at 11:26
  • Besides the error of using throw ex (or throw e) instead of just throw, I have to wonder what cases there are when it's worth catching an exception only to throw it again.
    – Ryan Lundy
    Aug 7, 2009 at 4:07
  • 13
    @Kyralessa: there are many cases: for instance, if you want to rollback a transaction, before the caller gets the exception. You rollback and then rethrow. Sep 10, 2009 at 8:55
  • 7
    I see this all the time where people catch and rethrow exceptions just because they are taught that they must catch all exceptions, not realising that it will be caught further up the call stack. It drives me nuts. May 5, 2010 at 12:55
  • 5
    @Kyralessa the biggest case is when you have to do logging. Log the error in catch, and rethrow..
    – nawfal
    Apr 10, 2013 at 11:53
145

Here's another time one that gets me:

static void PrintHowLong(DateTime a, DateTime b)
{
    TimeSpan span = a - b;
    Console.WriteLine(span.Seconds);        // WRONG!
    Console.WriteLine(span.TotalSeconds);   // RIGHT!
}

TimeSpan.Seconds is the seconds portion of the timespan (2 minutes and 0 seconds has a seconds value of 0).

TimeSpan.TotalSeconds is the entire timespan measured in seconds (2 minutes has a total seconds value of 120).

9
  • 1
    Yeah, that one has got me too. I think it should be TimeSpan.SecondsPart or something to make it more clear what it represents.
    – Dan Diplo
    Sep 10, 2009 at 9:51
  • 3
    On re-reading this, I have to wonder why TimeSpan even has a Seconds property at all. Who gives a rat's ass what the seconds portion of a timespan is, anyway? It's an arbitrary, unit-dependent value; I can't conceive of any practical use for it. Oct 8, 2010 at 22:48
  • 2
    Makes sense to me that TimeSpan.TotalSeconds would return... the total number of seconds in the time span.
    – Ed S.
    Feb 23, 2011 at 8:46
  • 16
    @MusiGenesis the property is useful. What if I want to display timespan broken out in pieces? E.g. let's say your Timespan represents duration of '3 hours 15 minutes 10 seconds'. How can you access this information without Seconds, Hours, Minutes properties? May 1, 2012 at 21:50
  • 1
    In similar APIs, I've used SecondsPart and SecondsTotal to distinguish the two. May 13, 2015 at 20:35
81

Leaking memory because you didn't un-hook events.

This even caught out some senior developers I know.

Imagine a WPF form with lots of things in it, and somewhere in there you subscribe to an event. If you don't unsubscribe then the entire form is kept around in memory after being closed and de-referenced.

I believe the issue I saw was creating a DispatchTimer in the WPF form and subscribing to the Tick event, if you don't do a -= on the timer your form leaks memory!

In this example your teardown code should have

timer.Tick -= TimerTickEventHandler;

This one is especially tricky since you created the instance of the DispatchTimer inside the WPF form, so you would think that it would be an internal reference handled by the Garbage Collection process... unfortunately the DispatchTimer uses a static internal list of subscriptions and services requests on the UI thread, so the reference is 'owned' by the static class.

6
  • 1
    The trick is to always release all event subscriptions you create. If you begin to rely on Forms doing it for you, you can be sure you'll get into the habit and one day will forget to release an event somewhere where it needs to be done. Jan 31, 2010 at 16:31
  • 3
    There is an MS-connect suggestion for weak reference events here which would solve this problem, though in my opinion we should just entirely replace the incredibly poor event model with a weakly-coupled one, like that used by CAB. Apr 28, 2010 at 16:39
  • +1 from me, thanks! Well, no thanks for the code review work I had to do!
    – Bob Denny
    May 2, 2010 at 18:34
  • @BlueRaja-DannyPflughoeft With weak events you have another gotcha - you cannot subscribe lambdas. You cannot write timer.Tick += (s, e,) => { Console.WriteLine(s); }
    – Ark-kun
    Oct 18, 2013 at 23:55
  • @Ark-kun yes lambdas make it even harder, you would have to save your lambda to a variable and use that in your teardown code. Kinda destroys the simplicity of writing lambdas doesn't it? Oct 20, 2013 at 22:07
63

Maybe not really a gotcha because the behavior is written clearly in MSDN, but has broken my neck once because I found it rather counter-intuitive:

Image image = System.Drawing.Image.FromFile("nice.pic");

This guy leaves the "nice.pic" file locked until the image is disposed. At the time I faced it I though it would be nice to load icons on the fly and didn't realize (at first) that I ended up with dozens of open and locked files! Image keeps track of where it had loaded the file from...

How to solve this? I thought a one liner would do the job. I expected an extra parameter for FromFile(), but had none, so I wrote this...

using (Stream fs = new FileStream("nice.pic", FileMode.Open, FileAccess.Read))
{
    image = System.Drawing.Image.FromStream(fs);
}
4
  • 11
    I agree that this behavior makes no sense. I can't find any explanation for it other than "this behavior is by design". Oct 15, 2009 at 16:04
  • 1
    Oh and what's great about this workaround is if you try to call Image.ToStream (I forget the exact name off hand) later it won't work.
    – Joshua
    Apr 28, 2010 at 18:09
  • 57
    need to check some code. Brb. Mar 12, 2012 at 6:39
  • 7
    @EsbenSkovPedersen Such a simple but funny & dry comment. Made my day.
    – Inisheer
    Jun 6, 2013 at 17:55
51

If you count ASP.NET, I'd say the webforms lifecycle is a pretty big gotcha to me. I've spent countless hours debugging poorly written webforms code, just because a lot of developers just don't really understand when to use which event handler (me included, sadly).

5
  • 26
    That's why I moved to MVC... viewstate headaches...
    – chakrit
    Oct 27, 2008 at 23:56
  • 29
    There was a whole other question devoted specifically to ASP.NET gotchas (deservedly so). The basic concept of ASP.NET (making web apps seem like windows apps for the developer) is so horribly misguided that I'm not sure it even counts as a "gotcha". Oct 28, 2008 at 0:53
  • 1
    MusiGenesis I wish I could up vote your comment a hundred times.
    – csauve
    Jul 22, 2010 at 16:45
  • 4
    @MusiGenesis It seems misguided now, but at the time, people wanted their web applications (applications being the key word - ASP.NET WebForms wasn't really designed to host a blog) to behave the same as their windows applications. This only changed relatively recently and a lot of people still "aren't quite there". The whole problem was that the abstraction was way too leaky - web didn't behave like a desktop application so much it lead to confusion in almost everyone.
    – Luaan
    Mar 27, 2014 at 16:40
  • 1
    Ironically enough, the first thing I ever saw about ASP.NET was a video from Microsoft demonstrating how easily you could create a blog site using ASP.NET! Mar 27, 2014 at 18:32
51

overloaded == operators and untyped containers (arraylists, datasets, etc.):

string my = "my ";
Debug.Assert(my+"string" == "my string"); //true

var a = new ArrayList();
a.Add(my+"string");
a.Add("my string");

// uses ==(object) instead of ==(string)
Debug.Assert(a[1] == "my string"); // true, due to interning magic
Debug.Assert(a[0] == "my string"); // false

Solutions?

  • always use string.Equals(a, b) when you are comparing string types

  • using generics like List<string> to ensure that both operands are strings.

8
  • 6
    You've got extra spaces in there which make it all wrong - but if you take the spaces out, the last line will still be true as "my" + "string" is still a constant.
    – Jon Skeet
    Oct 27, 2008 at 19:58
  • 1
    ack! you're right :) ok, I edited a bit.
    – Jimmy
    Oct 27, 2008 at 20:11
  • a warning is generated on such uses.
    – chakrit
    Oct 28, 2008 at 0:03
  • 11
    Yes, one of the biggest flaws with the C# language is the == operator in class Object. They should have forced us to use ReferenceEquals.
    – erikkallen
    Oct 11, 2009 at 13:57
  • 2
    Thankfully, since 2.0 we have had generics. There is less to worry about if you're using List<string> in the example above instead of ArrayList. Plus we've gained performance from it, yay! I'm always rooting out old references to ArrayLists in our legacy code.
    – JoelC
    Oct 9, 2014 at 19:31
50
[Serializable]
class Hello
{
    readonly object accountsLock = new object();
}

//Do stuff to deserialize Hello with BinaryFormatter
//and now... accountsLock == null ;)

Moral of the story : Field initialisers are not run when deserializing an object

1
  • 8
    Yes, I hate .NET serialization for not running the default constructor. I wish it were impossible to construct an object without calling any constructors, but alas it isn't. Apr 10, 2011 at 11:57
46

DateTime.ToString("dd/MM/yyyy"); This will actually not always give you dd/MM/yyyy but instead it will take into account the regional settings and replace your date separator depending on where you are. So you might get dd-MM-yyyy or something alike.

The right way to do this is to use DateTime.ToString("dd'/'MM'/'yyyy");


DateTime.ToString("r") is supposed to convert to RFC1123, which uses GMT. GMT is within a fraction of a second from UTC, and yet the "r" format specifier does not convert to UTC, even if the DateTime in question is specified as Local.

This results in the following gotcha (varies depending on how far your local time is from UTC):

DateTime.Parse("Tue, 06 Sep 2011 16:35:12 GMT").ToString("r")
>              "Tue, 06 Sep 2011 17:35:12 GMT"

Whoops!

5
  • 19
    Changed mm to MM - mm is minutes, and MM is months. Another gotcha, I guess...
    – Kobi
    Jul 8, 2009 at 11:17
  • 1
    I could see how this would be a gotcha if you didn't know it (I didn't)...but I'm trying to figure out when you would want the behavior where you're specifically trying to print a date that doesn't match what your regional settings are.
    – Beska
    Sep 8, 2009 at 16:01
  • 6
    @Beska: Because you are writing to a file, that needs to be in a specific format, with a specified date format.
    – GvS
    Mar 11, 2010 at 9:18
  • 11
    I am of the opinion that the defaults being localized is worse than the other way around. At least of the developer ignored localization completely the code works on machines localized differently. This way, the code probably doesn't work.
    – Joshua
    Apr 28, 2010 at 18:06
  • 32
    Actually I believe the correct way to do this would be DateTime.ToString("dd/MM/yyyy", CultureInfo.InvariantCulture); May 14, 2010 at 20:33
45

I saw this one posted the other day, and I think it is pretty obscure, and painful for those that don't know

int x = 0;
x = x++;
return x;

As that will return 0 and not 1 as most would expect

21
  • 38
    I hope that wouldn't actually bite people though - I really hope they wouldn't write it in the first place! (It's interesting anyway, of course.)
    – Jon Skeet
    Oct 27, 2008 at 19:34
  • 12
    I don't think this is very obscure... Oct 27, 2008 at 19:36
  • 11
    At least, in C#, the results are defined, if unexpected. In C++, it could be 0 or 1, or any other result including program termination! Oct 27, 2008 at 20:13
  • 7
    This isn't a gotcha; x=x++ -> x = x, then increment x....x=++x -> increment x then x = x
    – Kevin
    Oct 28, 2008 at 4:10
  • 30
    @Kevin: I don't think it's quite that simple. If x=x++ were equivalent to x=x followed by x++, then the result would be x = 1. Instead, I think what happens is first the expression to the right of the equals sign is evaluated (giving 0), then x is incremented (giving x = 1), and finally the assignment is performed (giving x = 0 once again). Jan 28, 2010 at 13:53
41

I'm a bit late to this party, but I have two gotchas that have both bitten me recently:

DateTime resolution

The Ticks property measures time in 10-millionths of a second (100 nanosecond blocks), however the resolution is not 100 nanoseconds, it's about 15ms.

This code:

long now = DateTime.Now.Ticks;
for (int i = 0; i < 10; i++)
{
    System.Threading.Thread.Sleep(1);
    Console.WriteLine(DateTime.Now.Ticks - now);
}

will give you an output of (for example):

0
0
0
0
0
0
0
156254
156254
156254

Similarly, if you look at DateTime.Now.Millisecond, you'll get values in rounded chunks of 15.625ms: 15, 31, 46, etc.

This particular behaviour varies from system to system, but there are other resolution-related gotchas in this date/time API.


Path.Combine

A great way to combine file paths, but it doesn't always behave the way you'd expect.

If the second parameter starts with a \ character, it won't give you a complete path:

This code:

string prefix1 = "C:\\MyFolder\\MySubFolder";
string prefix2 = "C:\\MyFolder\\MySubFolder\\";
string suffix1 = "log\\";
string suffix2 = "\\log\\";

Console.WriteLine(Path.Combine(prefix1, suffix1));
Console.WriteLine(Path.Combine(prefix1, suffix2));
Console.WriteLine(Path.Combine(prefix2, suffix1));
Console.WriteLine(Path.Combine(prefix2, suffix2));

Gives you this output:

C:\MyFolder\MySubFolder\log\
\log\
C:\MyFolder\MySubFolder\log\
\log\
6
  • 17
    The quantization of times in ~15ms intervals isn't because of a lack of accuracy in the underlying timing mechanism (I neglected to elaborate on this earlier). It's because your app is running inside a multi-tasking OS. Windows checks in with your app every 15ms or so, and during the little time slice it gets, your app processes all of the messages that were queued up since your last slice. All of your calls within that slice return the exact same time because they're all made at effectively the exact same time. Jul 17, 2009 at 1:53
  • 2
    @MusiGenesis: I know (now) how it works, but it seems misleading to me to have such a precise measure which isn't really that precise. It's like saying that I know my height in nanometres when really I'm just rounding it to the nearest ten million.
    – Damovisa
    Jul 17, 2009 at 2:25
  • 7
    DateTime is quite capable of storing up to a single tick; it's DateTime.Now that isn't using that accuracy.
    – Ruben
    Sep 21, 2009 at 23:31
  • 18
    The extra '\' is a gotcha to many unix/mac/linux folks. In Windows, if there's a leading '\', it's mean that we want to go the drive's root (i.e. C:) try it in a CD command to see what I mean.... 1) Goto C:\Windows\System32 2) Type CD \Users 3) Woah! Now you're at C:\Users ... GOT IT? ... Path.Combine(@"C:\Windows\System32", @"\Users") should returns \Users which means precisely the [current_drive_here]:\Users
    – chakrit
    Dec 28, 2009 at 14:04
  • 8
    Even without the 'sleep' this performs the same way. This has nothing to do with the app being scheduled every 15 ms. The native function called by DateTime.UtcNow, GetSystemTimeAsFileTime, appears to have a poor resolution.
    – Jimbo
    Apr 28, 2010 at 21:01
39

When you start a process (using System.Diagnostics) that writes to the console, but you never read the Console.Out stream, after a certain amount of output your app will appear to hang.

1
  • 3
    The same can still happen when you redirect both stdout and stderr and use two ReadToEnd calls in sequence. For safe handling of both stdout and stderr you have to create a read thread for each of them. Dec 6, 2009 at 11:42
36

No operator shortcuts in Linq-To-Sql

See here.

In short, inside the conditional clause of a Linq-To-Sql query, you cannot use conditional shortcuts like || and && to avoid null reference exceptions; Linq-To-Sql evaluates both sides of the OR or AND operator even if the first condition obviates the need to evaluate the second condition!

1
  • 9
    TIL. BRB, re-optimizing a few hundred LINQ queries...
    – tsilb
    Apr 19, 2013 at 22:26
31

Using default parameters with virtual methods

abstract class Base
{
    public virtual void foo(string s = "base") { Console.WriteLine("base " + s); }
}

class Derived : Base
{
    public override void foo(string s = "derived") { Console.WriteLine("derived " + s); }
}

...

Base b = new Derived();
b.foo();

Output:
derived base

11
  • 10
    Weird, I thought this is completely obvious. If the declared type is Base, where should the compiler get the default value from if not Base? I’d have thought it’s a bit more gotcha that the default value can be different if the declared type is the derived type, even though the method called (statically) is the base method.
    – Timwi
    Apr 10, 2011 at 12:27
  • 1
    why would one implementation of a method get the default value of another implementation?
    – staafl
    Jan 16, 2014 at 19:27
  • 1
    @staafl Default arguments are resolved at compile-time, not runtime. Mar 26, 2014 at 22:09
  • 2
    I'd say this gotcha is the default parameters in general - people often don't realize that they're resolved at compile-time, rather than run-time.
    – Luaan
    Mar 27, 2014 at 16:49
  • 4
    @FredOverflow, my question was conceptual. Although the behavior makes sense wrt the implementation, it's unintuitive and a likely source of errors. IMHO the C# compiler shouldn't allow changing default parameter values when overriding.
    – staafl
    Mar 27, 2014 at 18:41
28

Value objects in mutable collections

struct Point { ... }
List<Point> mypoints = ...;

mypoints[i].x = 10;

has no effect.

mypoints[i] returns a copy of a Point value object. C# happily lets you modify a field of the copy. Silently doing nothing.


Update: This appears to be fixed in C# 3.0:

Cannot modify the return value of 'System.Collections.Generic.List<Foo>.this[int]' because it is not a variable
3
  • 6
    I can see why that is confusing, considering that it does indeed work with arrays (contrary to your answer), but not with other dynamic collections, like List<Point>. Oct 27, 2008 at 22:08
  • 2
    You're right. Thanks. I fixed my answer :). arr[i].attr= is special syntax for arrays that you cannot code in library containers ;(. Why is (<value expression>).attr = <expr> allowed at all? Can it ever make sense? Oct 27, 2008 at 22:30
  • 1
    @Bjarke Ebert: There are some cases where it would make sense, but unfortunately there is no way for the compiler to identify and permit those. Sample usage scenario: an immutable Struct which holds a reference to a square two-dimensional array along with a "rotate/flip" indicator. The struct itself would be immutable, so writing to an element of a read-only instance should be fine, but the compiler won't know that the property setter isn't actually going to write the struct, and thus won't allow it.
    – supercat
    Feb 7, 2012 at 22:49
26

Perhaps not the worst, but some parts of the .net framework use degrees while others use radians (and the documentation that appears with Intellisense never tells you which, you have to visit MSDN to find out)

All of this could have been avoided by having an Angle class instead...

1
  • I'm surprised this got so many upvotes, considering my other gotchas are significantly worse than this Dec 17, 2010 at 18:25
22

For C/C++ programmers, the transition to C# is a natural one. However, the biggest gotcha I've run into personally (and have seen with others making the same transition) is not fully understanding the difference between classes and structs in C#.

In C++, classes and structs are identical; they only differ in the default visibility, where classes default to private visibility and structs default to public visibility. In C++, this class definition

    class A
    {
    public:
        int i;
    };

is functionally equivalent to this struct definition.

    struct A
    {
        int i;
    };

In C#, however, classes are reference types while structs are value types. This makes a BIG difference in (1) deciding when to use one over the other, (2) testing object equality, (3) performance (e.g., boxing/unboxing), etc.

There is all kinds of information on the web related to the differences between the two (e.g., here). I would highly encourage anyone making the transition to C# to at least have a working knowledge of the differences and their implications.

2
  • 13
    So, the worst gotcha is people not bothering to take the time to learn the language before they use it? Aug 2, 2011 at 20:43
  • 4
    @BlueRaja-DannyPflughoeft More like the classic gotcha of apparently similar languages - they use very similar keywords and in many cases syntax, but work in a lot different way.
    – Luaan
    Mar 27, 2014 at 16:48
19

Garbage collection and Dispose(). Although you don't have to do anything to free up memory, you still have to free up resources via Dispose(). This is an immensely easy thing to forget when you are using WinForms, or tracking objects in any way.

7
  • 2
    The using() block neatly solves this problem. Whenever you see a call to Dispose, you can immediately and safely refactor to use using(). Oct 27, 2008 at 20:21
  • 5
    I think the concern was implementing IDisposable correctly. Oct 27, 2008 at 23:16
  • 4
    On the other hand, the using() habit can bite you unexpectedly, like when working with PInvoke. You don't want to dispose something that the API is still referencing. Oct 28, 2008 at 0:50
  • 3
    Implementing IDisposable correctly is very hard to and understand even the best advice I have found on this (.NET Framework Guidelines) can be confusing to apply until you finally "get it". Oct 23, 2009 at 13:46
  • 1
    The best advice I ever found on IDisposable comes from Stephen Cleary, including three easy rules and an in-depth article on IDisposable Apr 10, 2011 at 12:00
19

Arrays implement IList

But don't implement it. When you call Add, it tells you that it doesn't work. So why does a class implement an interface when it can't support it?

Compiles, but doesn't work:

IList<int> myList = new int[] { 1, 2, 4 };
myList.Add(5);

We have this issue a lot, because the serializer (WCF) turns all the ILists into arrays and we get runtime errors.

11
  • 10
    IMHO, the problem is that Microsoft doesn't have enough interfaces defined for collections. IMHO, it should have iEnumerable, iMultipassEnumerable (supports Reset, and guarantees multiple passes will match), iLiveEnumerable (would have partially-defined semantics if the collection changes during enumeration--changes may or may not appear in enumeration, but shouldn't cause bogus results or exceptions), iReadIndexable, iReadWriteIndexable, etc. Because interfaces can "inherit" other interfaces, this wouldn't have added much extra work, if any (it would save NotImplemented stubs).
    – supercat
    Nov 19, 2010 at 0:08
  • @supercat, that would be confusing as hell for beginners and certain long-time coders. I think the .NET collections and their interfaces are wonderfully elegant. But I appreciate your humility. ;)
    – Jordan
    Jun 30, 2015 at 14:36
  • @Jordan: Since writing the above, I've decided that a better approach would have been to have both IEnumerable<T> and IEnumerator<T> support a Features property as well as some "optional" methods whose usefulness would be determined by what "Features" reported. I stand by my main point, though, which is that there are cases where code receiving an IEnumerable<T> will need stronger promises than IEnumerable<T> provides. Calling ToList would yield an IEnumerable<T> that upholds such promises, but would in many cases be needlessly expensive. I would posit that there should be...
    – supercat
    Jun 30, 2015 at 15:29
  • ...a means by which code receiving an IEnumerable<T> could make a copy of the contents if needed but could refrain from doing so needlessly.
    – supercat
    Jun 30, 2015 at 15:31
  • Your option is absolutely not readable. When I see an IList in code I know what I am working with rather than having to probe a Features property. Programmers like to forget that an important feature of code is that it can be read by people not just computers. The .NET collections namespace is not ideal but it is good, and sometimes finding the best solution is not a matter of fitting a principle more ideally. Some of the worst code I have every worked with was code that tried to fit DRY ideally. I scrapped it and rewrote it. It was just bad code. I would not want to use your framework at all.
    – Jordan
    Jun 30, 2015 at 17:35
18

foreach loops variables scope!

var l = new List<Func<string>>();
var strings = new[] { "Lorem" , "ipsum", "dolor", "sit", "amet" };
foreach (var s in strings)
{
    l.Add(() => s);
}

foreach (var a in l)
    Console.WriteLine(a());

prints five "amet", while the following example works fine

var l = new List<Func<string>>();
var strings = new[] { "Lorem" , "ipsum", "dolor", "sit", "amet" };
foreach (var s in strings)
{
    var t = s;
    l.Add(() => t);
}

foreach (var a in l)
    Console.WriteLine(a());
6
  • 12
    This is essentially equivalent to Jon's example with anonymous methods. Aug 25, 2009 at 0:08
  • 3
    Save that it is even more confusing with foreach where the "s" variable is easier to mix with scoped variable. With common for-loops the index variable clearly is the same one for each iteration. Dec 6, 2009 at 14:55
  • 2
    blogs.msdn.com/ericlippert/archive/2009/11/12/… and yes, wish the variable was scoped "properly". Jan 12, 2010 at 23:40
  • 3
    This was fixed in C# 5.
    – Johnbot
    May 12, 2015 at 20:37
  • 1
    @Johnbot Not fixed, changed.
    – IS4
    Jul 12, 2016 at 14:38
18

MS SQL Server can't handle dates before 1753. Significantly, that is out of synch with the .NET DateTime.MinDate constant, which is 1/1/1. So if you try to save a mindate, a malformed date (as recently happened to me in a data import) or simply the birth date of William the Conqueror, you're gonna be in trouble. There is no built-in workaround for this; if you're likely to need to work with dates before 1753, you need to write your own workaround.

6
  • 17
    Quite frankly I think MS SQL Server has this right and .Net is wrong. If you do the research then you know that dates prior to 1751 get funky due to calendar changes, days completely skipped, etc. Most RDBMs have some cut off point. This should give you a starting point: ancestry.com/learn/library/article.aspx?article=3358
    – NotMe
    Oct 8, 2009 at 15:22
  • 11
    Also, the date is 1753.. Which was pretty much the first time that we have a continuous calendar without dates being skipped. SQL 2008 introduced the Date and datetime2 datetype which can accept dates from 1/1/01 to 12/31/9999. However, date comparisons using those types should be viewed with suspicion if you are really comparing pre-1753 dates.
    – NotMe
    Oct 8, 2009 at 15:27
  • Oh, right, 1753, corrected, thanks.
    – Shaul Behr
    Oct 11, 2009 at 13:31
  • Does it really make sense to do date comparisons with such dates? I mean, for History Channel this makes a lot of sense, but I don't see myself wanting to know the precise day of the week America was discovered. Feb 16, 2013 at 5:58
  • 5
    Through Wikipedia on Julian Day you can find a 13 line basic program CALJD.BAS published in 1984 that can do date calculations back to about 5000 BC, taking into account leap days and the skipped days in 1753. So I do not see why "modern" systems like SQL2008 should do worse. You might not be interested in a correct date representation in the 15th century, but others might, and our software should handle this without bugs. Another issue is leap seconds . . .
    – Roland
    Aug 23, 2013 at 7:58
18

The contract on Stream.Read is something that I've seen trip up a lot of people:

// Read 8 bytes and turn them into a ulong
byte[] data = new byte[8];
stream.Read(data, 0, 8); // <-- WRONG!
ulong data = BitConverter.ToUInt64(data);

The reason this is wrong is that Stream.Read will read at most the specified number of bytes, but is entirely free to read just 1 byte, even if another 7 bytes are available before end of stream.

It doesn't help that this looks so similar to Stream.Write, which is guaranteed to have written all the bytes if it returns with no exception. It also doesn't help that the above code works almost all the time. And of course it doesn't help that there is no ready-made, convenient method for reading exactly N bytes correctly.

So, to plug the hole, and increase awareness of this, here is an example of a correct way to do this:

    /// <summary>
    /// Attempts to fill the buffer with the specified number of bytes from the
    /// stream. If there are fewer bytes left in the stream than requested then
    /// all available bytes will be read into the buffer.
    /// </summary>
    /// <param name="stream">Stream to read from.</param>
    /// <param name="buffer">Buffer to write the bytes to.</param>
    /// <param name="offset">Offset at which to write the first byte read from
    ///                      the stream.</param>
    /// <param name="length">Number of bytes to read from the stream.</param>
    /// <returns>Number of bytes read from the stream into buffer. This may be
    ///          less than requested, but only if the stream ended before the
    ///          required number of bytes were read.</returns>
    public static int FillBuffer(this Stream stream,
                                 byte[] buffer, int offset, int length)
    {
        int totalRead = 0;
        while (length > 0)
        {
            var read = stream.Read(buffer, offset, length);
            if (read == 0)
                return totalRead;
            offset += read;
            length -= read;
            totalRead += read;
        }
        return totalRead;
    }

    /// <summary>
    /// Attempts to read the specified number of bytes from the stream. If
    /// there are fewer bytes left before the end of the stream, a shorter
    /// (possibly empty) array is returned.
    /// </summary>
    /// <param name="stream">Stream to read from.</param>
    /// <param name="length">Number of bytes to read from the stream.</param>
    public static byte[] Read(this Stream stream, int length)
    {
        byte[] buf = new byte[length];
        int read = stream.FillBuffer(buf, 0, length);
        if (read < length)
            Array.Resize(ref buf, read);
        return buf;
    }
1
  • 1
    Or, in your explicit example: var r = new BinaryReader(stream); ulong data = r.ReadUInt64();. BinaryReader has a FillBuffer method too... Jan 13, 2014 at 16:02
18

The Nasty Linq Caching Gotcha

See my question that led to this discovery, and the blogger who discovered the problem.

In short, the DataContext keeps a cache of all Linq-to-Sql objects that you have ever loaded. If anyone else makes any changes to a record that you have previously loaded, you will not be able to get the latest data, even if you explicitly reload the record!

This is because of a property called ObjectTrackingEnabled on the DataContext, which by default is true. If you set that property to false, the record will be loaded anew every time... BUT... you can't persist any changes to that record with SubmitChanges().

GOTCHA!

2
  • Iv just spent a day and a half (and loads of hair!) chasing down this BUG... Dec 3, 2009 at 16:26
  • This is called a concurrency conflict and it is still a gotcha today even though there are certain ways around this now though they tend to be a bit heavy handed. DataContext was a nightmare. O_o
    – Jordan
    Jun 30, 2015 at 14:27
16

Events

I never understood why events are a language feature. They are complicated to use: you need to check for null before calling, you need to unregister (yourself), you can't find out who is registered (eg: did I register?). Why isn't an event just a class in the library? Basically a specialized List<delegate>?

6
  • 1
    Also, multithreading is painful. All these issues but the null-thing are fixed in CAB (whose features should really just be built into the language) - events are declared globally, and any method can declare itself to be a "subscriber" of any event. My only issue with CAB is that the global event names are strings rather than enums (which could be fixed by more intelligent enums, like Java has, which inherently work as strings!). CAB is difficult to set up, but there is a simple open-source clone available here. Jul 9, 2010 at 14:32
  • 4
    I dislike the implementation of .net events. Event subscription should be handled by calling a method that adds the subscription and returns an IDisposable which, when Dispose'd, will delete the subscription. There's no need for a special construct combining an "add" and "remove" method whose semantics can be somewhat dodgy, especially if one attempts to add and later remove a multicast delegate (e.g. Add "B" followed by "AB", then remove "B" (leaving "BA") and "AB" (still leaving "BA"). Oops.
    – supercat
    Dec 21, 2011 at 1:07
  • @supercat How would you rewrite button.Click += (s, e) => { Console.WriteLine(s); }?
    – Ark-kun
    Oct 19, 2013 at 0:14
  • If I would have to be able to unsubscribe separately from other events, IEventSubscription clickSubscription = button.SubscribeClick((s,e)=>{Console.WriteLine(s);}); and unsubscribe via clickSubscription.Dispose();. If my object would keep all subscriptions throughout its lifetime, MySubscriptions.Add(button.SubscribeClick((s,e)=>{Console.WriteLine(s);})); and then MySubscriptions.Dispose() to kill all subscriptions.
    – supercat
    Oct 19, 2013 at 0:21
  • @Ark-kun: Having to keep objects that encapsulate outside subscriptions might seem like a nuisance, but regarding subscriptions as entities would make it possible to aggregate them with a type that can ensure they all get cleaned up, something which is otherwise very difficult.
    – supercat
    Oct 19, 2013 at 0:27
16

Today I fixed a bug that eluded for long time. The bug was in a generic class that was used in multi threaded scenario and a static int field was used to provide lock free synchronisation using Interlocked. The bug was caused because each instantiation of the generic class for a type has its own static. So each thread got its own static field and it wasn't used a lock as intended.

class SomeGeneric<T>
{
    public static int i = 0;
}

class Test
{
    public static void main(string[] args)
    {
        SomeGeneric<int>.i = 5;
        SomeGeneric<string>.i = 10;
        Console.WriteLine(SomeGeneric<int>.i);
        Console.WriteLine(SomeGeneric<string>.i);
        Console.WriteLine(SomeGeneric<int>.i);
    }
}

This prints 5 10 5

3
  • 6
    you can have a non-generic base class, that defines the statics, and inherit the generics from it. Although I never fell for this behavior in C# - I still remember the long debugging hours of some C++ templates... Eww! :)
    – Paulius
    Sep 10, 2009 at 10:09
  • 7
    Weird, I thought this was obvious. Just think about what it should do if i had the type T.
    – Timwi
    Apr 10, 2011 at 12:10
  • 1
    The type parameter is part of the Type. SomeGeneric<int> is a different Type from SomeGeneric<string>; so of course each has its own public static int i
    – radarbob
    Nov 6, 2015 at 22:32
14

Just found a weird one that had me stuck in debug for a while:

You can increment null for a nullable int without throwing an excecption and the value stays null.

int? i = null;
i++; // I would have expected an exception but runs fine and stays as null
1
  • That's the result of how C# leverages operations for nullable types. It is a bit similar to NaN consuming all you throw to it.
    – IS4
    Feb 23, 2016 at 15:35
13

Enumerables can be evaluated more than once

It'll bite you when you have a lazily-enumerated enumerable and you iterate over it twice and get different results. (or you get the same results but it executes twice unnecessarily)

For example, while writing a certain test, I needed a few temp files to test the logic:

var files = Enumerable.Range(0, 5)
    .Select(i => Path.GetTempFileName());

foreach (var file in files)
    File.WriteAllText(file, "HELLO WORLD!");

/* ... many lines of codes later ... */

foreach (var file in files)
    File.Delete(file);

Imagine my surprise when File.Delete(file) throws FileNotFound!!

What's happening here is that the files enumerable got iterated twice (the results from the first iteration are simply not remembered) and on each new iteration you'd be re-calling Path.GetTempFilename() so you'll get a different set of temp filenames.

The solution is, of course, to eager-enumerate the value by using ToArray() or ToList():

var files = Enumerable.Range(0, 5)
    .Select(i => Path.GetTempFileName())
    .ToArray();

This is even scarier when you're doing something multi-threaded, like:

foreach (var file in files)
    content = content + File.ReadAllText(file);

and you find out content.Length is still 0 after all the writes!! You then begin to rigorously checks that you don't have a race condition when.... after one wasted hour... you figured out it's just that tiny little Enumerable gotcha thing you forgot....

4
  • This is by design. It's called deferred execution. Among other things, it's meant to simulate TSQL constructs. Every time you select from a sql view you get different results. It also allows chaining which is helpful for remote data stores, such as SQL Server. Otherwise x.Select.Where.OrderBy would send 3 separate commands to the database...
    – as9876
    Jul 14, 2015 at 3:50
  • @AYS did you miss the word "Gotcha" in the question title?
    – chakrit
    Jul 14, 2015 at 4:26
  • I thought gotcha meant an oversight of the designers, not something intentional.
    – as9876
    Jul 14, 2015 at 4:32
  • Maybe there should be another type for non-restartable IEnumerables. Like, AutoBufferedEnumerable? One could implement it easily. This gotcha seem mostly due to the programmer's lack of knowledge, I don't think there's anything wrong with the current behavior. Jun 16, 2016 at 11:22
10
TextInfo textInfo = Thread.CurrentThread.CurrentCulture.TextInfo;

textInfo.ToTitleCase("hello world!"); //Returns "Hello World!"
textInfo.ToTitleCase("hElLo WoRld!"); //Returns "Hello World!"
textInfo.ToTitleCase("Hello World!"); //Returns "Hello World!"
textInfo.ToTitleCase("HELLO WORLD!"); //Returns "HELLO WORLD!"

Yes, this behavior is documented, but that certainly doesn't make it right.

2
  • 6
    I disagree - when a word is in all caps, it can have special meaning that you don't want to mess up with Title Case, e.g. "president of the USA" -> "President Of The USA", not "President Of The Usa".
    – Shaul Behr
    May 5, 2010 at 12:56
  • 6
    @Shaul: In which case, they should specify this as a parameter to avoid confusion, because I've never met anyone who expected this behaviour ahead of time - which makes this a gotcha! May 5, 2010 at 13:18

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