Ignoring the Dispose Method
This is by far the worst thing that happens to many juniors. The IDisposable interface is implemented in many classes. This interface has a single method called Dispose. This method is used to free unmanaged resources like database connections, file streams, or unmanaged memory.
In C# you can call directly the Dispose method or you can use a using block.
using (SqlConnection sqlConnection= new SqlConnection("connectionsstring"))
{
sqlConnection.Query("Query the database");
} // Dispose method is called automatically when the block is exited
Since C# version 8, you can use using declarations like this.
using SqlConnection sqlConnection= new SqlConnection("connectionsstring");
sqlConnection.Query("Query the database");
// Dispose method is called automatically when the variable sqlconnection is no longer used
The using statement will translate in a try/finally block. The using statement will automatically dispose the resources, even if an exception occurs.
The easiest option to identify the objects that are not disposed of is to use an extension like DisposableFixer. You can also use this code analyzer.
Not Handling Exceptions Correctly
Another pitfall that causes bad error logging is not handling exceptions correctly.
I have seen many programmers writing this type of code:
try
{
//some code
FileController file = new FileController("path");
file.Open(FileAccessMode.New);
//
}
catch(FileException ex)
{
logger.log(ex);
throw ex; // don't do this. The stack trace will not preserve and in your logs, you will see the exception was thrown from this line.
}
catch(Exception ex)
{
throw new Exception(ex); // don't do this
}
Whenever you need to rethrow the exception, just use the keyword throw:
catch(Exception ex)
{
throw;
}
The stack trace will be preserved.
Overuse of Boxing and Unboxing
Boxing represents the concept of converting a value into an object reference, while unboxing represents the process of extracting the value type from the object.
If you overuse these concepts you will lead to performance issues and memory consumption. This happens because you allocate memory on the heap.
ArrayList myList = new ArrayList();
for (int i = 0; i < 999999; i++)
{
myList.Add(i); // Boxing occurs here
}
foreach (object item in myList)
{
int value = (int)item; // Unboxing occurs here
}
To solve this issue, try to use collections that have homogeneous elements. Try to use generic collections like List or Dictionary.
Using the wrong collection type
If you are dealing with a large amount of data, you must use the correct collection.
In C#, the most popular collections are:
- Arrays
- Lists
- Dictionaries
- Queues
- Stacks
- LinkedList
- HashSet
- SortedList
- ArrayList
- HashTable
Some types of collections are faster but may require more memory. For example, the HashSet class offers faster operations than List, but this comes with a higher memory consumption.
Every situation is different, sometimes you need collections as fast as possible, and other times you need ones that don’t require a lot of memory.
When you use threads make sure that you use concurrent collections like Concurrent Bag or Concurrent Dictionary.
Choosing ArrayList instead of List and using HashTable instead of Dictionary when you have elements of the same type.
ArrayList and HashTable are older and non-generic collections. Try to use generic collections when you can.
Using List for Large Datasets that requires insertion and deletion
The List class doesn’t have methods to insert elements in the middle of the list. Use instead LinkedList class.
Using Dictionary for Sequential Data
Dictionaries are good when you have key-value access. When you need to access data sequentially, try to use Lists, Sorted Lists, or Queues.
String Concatenation
Don’t use string concatenation in a loop because you will end up using a lot of memory. Use instead the StringBuilder class.
using StreamReader stream = new StreamReader("TextFile1.txt");
string text = "";
while (!stream.EndOfStream)
{
text += stream.ReadLine().Replace("|", ","); // at every iteration a new reference to text is created
}
Console.WriteLine(text)
// Do this
using StreamReader stream = new StreamReader("TextFile1.txt");
StringBuilder stringBuilder = new StringBuilder();
while (!stream.EndOfStream)
{
stringBuilder.AppendLine(stream.ReadLine().Replace("|", ","));
}
Console.WriteLine(stringBuilder.ToString());
Reading all the content instead of using streams
Usually when you deal with small files is ideal to use File.ReadAllLines method. When you deal with big files, I recommend you avoid this method because it loads all the file’s content into memory. Usually, you don’t need to load the entire file in memory.
Not using asynchronous operations for Input and Output operations
Async and await keywords are used to implement asynchronous operations. These types of operations are needed for long-running tasks that imply blocking the thread like reading a file, sending an HTTP request, or querying the database.
// Synchronous I/O operation
string text= File.ReadAllText("file.txt");
// Asynchronous I/O operation
string text= await File.ReadAllTextAsync("file.txt");
In a desktop application, async and await help you avoid freezing the UI thread. In a web application, it helps you to handle more requests.
For tasks that run CPU-intensive computations try to use Parallel class. I have written an article that tells you when to use Parallel classes instead of asynchronous tasks.
LINQ Queries Pitfalls
LINQ offers you query capabilities directly from C#. This can be tricky if you don’t know how to use it. You can improve or unnecessarily load a server with useless data.
Here are some examples of bad queries and how you can improve it:
Using Any Instead of Contains for Collections
Don’t use Any if you can use the Contains method.
var userIds = new List<int> { 1, 2, 3 };
var users = dbContext.Users.Where(u => userIds.Any(id => id == u.Id)).ToList(); // Don't
var users = dbContext.Users.Where(u => userIds.Contains(u.Id)).ToList(); // Do
Retrieving unnecessary data
Don’t retrieve the entire object if you don’t need it, especially when you load many rows. Use the Select method and get only the properties that you need.
_context.Animals.ToList(); // don't retrieve the entire object _context.Animals.Select(a=>a.Name).ToList(); // retrieve only the needed data
Using ToList too early
The call of the method ToList must be the last. You don’t call the sort method for example to order the objects after you retrieve it from the database.
var recentOrders = dbContext.Orders
.Where(o => o.OrderDate > DateTime.Now.AddMonths(-3))
.ToList() // don't call it here!
.OrderByDescending(o => o.OrderDate)
.ToList();
// do this instead
var recentOrders = dbContext.Orders
.Where(o => o.OrderDate > DateTime.Now.AddMonths(-3))
.OrderByDescending(o => o.OrderDate)
.ToList();
Not Using Enums
When you are dealing with magic strings you should use enums instead.
Magic strings are hardcoded strings that are usually used for configuration.
PictureQuality pictureQuality= new PictureQuality("Medium");
// ..
if(pictureQuality.Compression=="Medium") // bad design
{
}
Magic strings are not good because they can easily break the code. For example, maybe you want to rename the compression setting medium to something else. Then you need to rename it manually in the entire application. It can also break the application if someone misspells the medium setting.
PictureQuality pictureQuality= new PictureQuality("Medium");
// ..
if(pictureQuality.Compression=="Medum") // bad design
{
}
If you use enums, you avoid these types of errors. You will have type safety out of the box.
PictureQuality pictureQuality= new PictureQuality(Compression.Medium);
// ..
if(pictureQuality.Compression==Compression.Medium){
}
Overusing var keyword
Don’t use the var keyword too much because it can reduce the readability of the code.
var result = Method(); // what does Method returns? long result = Method(); // the preffered way
Overusing Regions
I have seen programmers that use regions to wrap their methods, another region for properties and another one for fields. Don’t ever use the regions like this. Regions are good for wrapping code that is generated automatically or code that is not usually modified.
If you deal with a codebase that have a lot of regions, you can use an extension for Visual Studio that expands automaticall all the regions.
