.NET Development and C# in particular have come a long way over the years. I've been using it since beta 2 in 2001 and I love it, but there are some common gotchas that are easy to make when you're just getting started.
Exceptions
Throwing Exception
If you need to throw an Exception, don't do the following:
throw new Exception("Customer cannot be null");
Instead do something more targeted to your exception case:
throw new ArgumentNullException(nameof(customer), "Customer cannot be null");
The reason why this is important is explained in the next section.
Catching Exception
When a generic Exception instance is thrown and you want to be able to handle that, you're forced to do something like the following:
try {
DoSomethingRisky();
} catch (Exception ex) {
if (ex.Message == "Customer cannot be null") {
// Some specific logic goes here
} else {
// Another handler or rethrow
}
}
Catching Exception catches all forms of Exceptions and is rarely ever what you actually should do. Instead, you should be looking for a few targeted types of exceptions that you expect based on what you're calling and should have handlers for those, letting more unexpected exceptions bubble up.
try {
DoSomethingRisky();
} catch (ArgumentNullException ex) {
// Some specific logic goes here
}
Rethrowing Exceptions
When catching an exception, you sometimes want to rethrow it - particularly if it doesn't match a specific criteria. The syntax for correctly rethrowing an exception is different than you'd expect since it's different than originally throwing an exception.
Instead of:
try {
DoSomethingRisky();
} catch (InvalidOperationException ex) {
if (ex.Message.StartsWith("Cannot access the")) {
// Some specialized handling logic
} else {
throw ex; // We don't have a specific solution for this. Rethrow it
}
}
Do:
try {
DoSomethingRisky();
} catch (InvalidOperationException ex) {
if (ex.Message.StartsWith("Cannot access the")) {
// Some specialized handling logic
} else {
throw; // The ex is implicit
}
}
The reason you need to do this is because of how .NET stack traces work. You want to retain the original stack trace instead of making the exception look like a new exception in the catch block. If you are instead using throw ex (or similar) you'll miss some of the original context of the exception.
Design
Working with Immutable Types
Some types, like DateTime are said to be immutable, in that you can create one, but you cannot change it after creation. These classes expose methods that allow you to perform operations that create a new instance based on their own data, but these methods do not alter the object they are invoked on and this can be misleading.
For example, with a DateTime, if you were trying to advance a tracking variable by a day, you would not do this:
myMeeting.Date.AddDays(1);
This statement would execute and run without error, but the value of myMeeting.Date would remain what it originally was since AddDays returns the new value instead of modifying the existing object.
To change myMeeting.Date, you would instead do the following:
myMeeting.Date = myMeeting.Date.AddDays(1);
TimeSpan Properties
Speaking of Dates, TimeSpan exposes some interesting properties that might be misleading. For example, if you looked at a TimeSpan, you might be tempted to look at the milliseconds to see how long something took, but if it took a second or longer, you're only going to get the milliseconds component for display purposes, not the total milliseconds.
Don't do this:
TimeSpan result = myStopWatch.Elapsed;
Console.Debug("The operation took " + result.Milliseconds + " ms");
Instead, use the TotalX series of methods like this:
TimeSpan result = myStopWatch.Elapsed;
Console.Debug("The operation took " + result.TotalMilliseconds + " ms");
Double Comparison
When comparing doubles, it's easy to think that you could just do:
bool areEqual = myDouble == myOtherDouble;
But due to the sensitivity of double mathematics, those numbers could be slightly off when dealing with fractions.
Instead, either use the decimal class or compare that the numbers are extremely close by using an Epsilon:
bool areEqual = Math.Abs(myDouble - myOtherDouble) < Double.Epsilon;
Frankly, I tend to steer away from double in favor of decimal to avoid syntax like this.
Misc
String Appending
When working with strings, it can be performance intensive to do a large amount of string appending logic, since new strings need to be created for each combination encountered along the way, leading to higher frequencies of garbage collection and noticeable performance spikes.
If you're in a scenario where you expect to append to a string more than 3 times on average, you should instead use a StringBuilder which does techno ninja voo doo trickery internally to optimize the memory overhead for building a string from smaller strings.
Instead of:
string numbers = "";
for (int i = 0; i < 1000; i++) {
numbers += i;
}
return numbers;
Do:
StringBuilder sb = new StringBuilder();
for (int i = 0; i < 1000; i++) {
sb.Append(i);
}
return sb.ToString(); // Actually builds the final string
Using Statements
When working with IDisposable instances, it's important to make sure that Dispose is properly called - including in cases when exceptions are encountered. Failing to dispose something like a SqlConnection can lead to instances where databases do not have available connections for new requests, which brings production servers to a sudden halt.
Instead of:
var conn = new SqlConnection(dbConnStr);
conn.Open();
// Do some things that could throw errors
conn.Close();
do this:
using (var conn = new SqlConnection(dbConnStr)) {
conn.Open();
// Do some things that could throw errors
} // Note that IDisposable will take care of closing the connection if active
This is the equivalent of a try / finally that calls Dispose() on conn if conn is not null. Note also that database adapters will close connections as part of their IDisposable implementation.
Overall using leads to cleaner and safer code.
Async Void
When you declare an asynchronous method that doesn't return anything, syntactically it's tempting to declare it as follows:
public async void SendEmailAsync(EmailInfo email) { /* ... */ }
However, if an exception occurs, the information will not correctly propagate to the caller due to how the threading logic works under the cover. This means that any try / catch logic in the caller won't work the way you expect and you'll have buggy exception handling behavior.
Instead do this:
public async Task SendEmailAsync(EmailInfo email) { /* ... */ }
The Task return type allows .NET to send exception information back to the caller as you would normally expect.
Preprocessor Statements
Preprocessor statements are just plain old bad. To those unfamiliar, a preprocessor statement allows you to do actions prior to compilation based things defined or not defined in your build options.
bool allowClipboardUse = true;
#if SILVERLIGHT
allowClipboardUse = false; // Browser security is fun
// Also, probably weeping and switching to Angular
#end if
The correct use of preprocessor statements is for environment-specific things, such as using a library for x64 architecture instead of another one for x86 architecture, or including some logic for mobile applications but not for other applications sharing the same code.
The problem is that people take this capability and try to bake in customer-specific logic, effectively fragmenting the code for allowing it to compile targeting different targets, but by which set of logical rules or UI styling is desired.
This becomes hard to maintain and hard to test and does not scale well. It also makes it easy to introduce errors while refactoring and overall will slow your team's velocity over time.
Some people advocate for using the DEBUG preprocessor definition to allow for testing logic on local development copies, but be very careful with this. I once encountered a production bug related to deserialization where the development version worked fine every time because it had a property setter defined in a DEBUG preprocessor statement, but deserialization failed in production for that field leading to buggy behavior.
Again, be very careful and lean towards object-oriented patterns like the Strategy or Command pattern for client-specific logic or other types of behavioral logic.
Deserialization
Speaking of deserialization, be mindful of private variables, properties without setters, and logic that exists in property setters or getters. Different serialization / deserialization libraries approach things differently, but these areas tend to introduce obscure bugs where properties will not populate correctly during deserialization.
These are a few of the common .NET mistakes I see people encounter. What others are there out there that I neglected to mention?
Top comments (8)
This is a really useful article— I learned a couple new things!
It got me thinking about a couple others off the top of my head...
1) Junior Devs seemingly are never taught how to correctly encode strings for URLs, Json, XML, etc (spoiler, string concatenation is not reliable for any of these).
2) On the topic of correctly handling IDisposables such as Streams....a byte[] should be favored over a MemoryStream when passing around data among functions, so that Streams are always used locally in the method that needs it and disposed of immediately (eg using{})...And while on this note, that bright idea that Dev for my client had about converting that byte[] to a base64 encoded string to pass around between functions —which then decodes it over and over— was a pretty bad idea!
3) Junior Devs need stop abusing ORM libraries...yeah occasionally it helps develop slightly quicker in small apps, but tightly coupling all Models with Table names and column names is the first yellow flag for enterprise apps. But the performance issue with in-code-looping-joins is beyond aweful....just learn Sql, create some views, and let Sql Server out of the ORM cage (sql server performance can be blazingly fast if you do it right from the beginning).
I love all of these. Thank you for sharing them. I also should have included something on storing dates in UTC. That's more data-model related, but it's another very common mistake I see alongside encoding.
It seems like a good idea to ensure streams are disposed of during functions, but I can't put my finger on exactly why - would it be possible to elaborate?
P.S. good additions.
Fantastic post Matt - really enjoyed it! with goldies like
Double.Epsilonand "Preprocessor statements are just plain old bad."They're evil. It's like GOTO was when I was a kid - it just makes it so easy for programmers to do things they really shouldn't. Thanks for the feedback. I've given a training presentation internally to my company on this topic and found it valuable enough to share some concepts from that with the industry as a whole.
Excellent post with important topics that need to be obeyed when building an API!
I have worked a lot with immutable DateTime type; however, your point on it was really amazing and helpful for me. Thanks!
Great article! I learned a lot specially the "Double Comparison" which is completely new to me. Looks like StackOverflow has been hiding this for a very long time now. 🤣👍