7 Common Mistakes Made By C# Programmers

Introduction

Making mistakes is inevitable in programming. Even a small mistake could prove to be very costly. The wise thing is to learn from your mistakes and try not to repeat them. In this article I will be highlighting the mistakes which I consider to be the 7 most common mistakes made by C# developers.

Formatting a string

There lies the strong possibility of making costly mistakes while working with string types in C# programming. String is always an immutable type in the .NET Framework. When a string is modified it always creates a new copy and never changes the original. Most developers always format the string as shown in the sample below:

  string updateQueryText = "UPDATE EmployeeTable SET Name='" + name + "' WHERE EmpId=" + id;

The above code is really messy and also as the string is immutable it creates 3 unnecessary garbage string copies in the memory as a result of multiple concatenations.

The better approach is to use string.Format as it internally uses StringBuilder which is mutable. It also paves the way for a clean code.

  string updateQueryText = string.Format("UPDATE EmployeeTable SET Name='{0}' WHERE EmpId={1}", name, id);

Nested Exception Handling

Developers who intend to write nested methods end up doing exception handling for each methods as shown in the below code:

  public class NestedExceptionHandling
  {
      public void MainMethod()
      {
          try
          {
              //some implementation
              ChildMethod1();
          }
          catch (Exception exception)
          {
              //Handle exception
          }
      }
  
      private void ChildMethod1()
      {
          try
          {
              //some implementation
              ChildMethod2();
          }
          catch (Exception exception)
          {
              //Handle exception
  	     throw;
  
          }
      }
  
      private void ChildMethod2()
      {
          try
          {
              //some implementation
          }
          catch (Exception exception)
          {
              //Handle exception
              throw;
          }
      }
  }

So what would happen with the above code if the same exception were handled multiple times and more over catching exceptions, throwing it will definitely add a performance overhead.

I try to avoid this by putting the exception handling in a single place that is in the MainMethod as shown below:

  public class NestedExceptionHandling
  {
      public void MainMethod()
      {
          try
          {
              //some implementation
              ChildMethod1();
          }
          catch(Exception exception)
          {
              //Handle exception
          }
      }
  
      private void ChildMethod1()
      {
          //some implementation
          ChildMethod2();
      }
  
      private void ChildMethod2()
      {
          //some implementation
      }
  }

Using Foreach on Huge Collections

Most developers prefer to use a foreach loop than for loop because foreach is easier to use. This will however prove to be costly when working with collections with a large amount of data. Consider the below sample in which I am looping through the same datatable using for and foreach. Also check the time taken by each loop on Fig 1.0.

  static void Main(string[] args)
  {
      DataTable dt = PopulateData();
      Stopwatch watch = new Stopwatch();
  
      //For loop
      watch.Start();
      for (int count = 0; count < dt.Rows.Count; count++)
      {
          dt.Rows[count]["Name"] = "Modified in For";
      }
      watch.Stop();
      Console.WriteLine("Time taken in For loop: {0}", watch.Elapsed.TotalSeconds);
  
      //Foreach loop
      watch.Start();
      foreach (DataRow row in dt.Rows)
      {
          row["Name"] = "Modified in ForEach";
      }
      watch.Stop();
      Console.WriteLine("Time taken in For Each loop: {0}", watch.Elapsed.TotalSeconds);
  
      Console.ReadKey();
  }

time taken by each loop
Fig 1.0

As you can see the foreach loop is slow, it takes almost double the amount of time as that of the for loop. This is because in the foreach loop dt.Rows will access all the rows in the datatable.

For bigger collections always use for loop in case if looping is required.



7 Common Mistakes Made By C# Programmers

Validating Simple Primitive Data Types

Most developers are not aware of the inbuilt methods available for validating the primitive data types like System.Int32 and end up doing a custom implementation. The function below is a sample of that custom implementation to validate whether the given string is numeric or not.

  public bool CheckIfNumeric(string value)
  {
      bool isNumeric = true;
      try
      {
          int i = Convert.ToInt32(value);
      }
      catch(FormatException exception)
      {
          isNumeric = false;
      }
      return isNumeric;
  }

Since it involves try catch it is not the best way. A better approach would be to use int.TryParse as shown below:

  int output = 0;
  bool isNumeric = int.TryParse(value, out output);

int.TryParse, in my opinion is definitely the faster and cleaner way.

Handling Objects Implementing IDisposable Interface

In the .NET Framework disposing of an object is as important as consuming it. The ideal approach would be to implement the IDisposable interface's dispose method in the class, so after using the object of that class, it can be disposed by calling the dispose method.

Below is a sample where an SqlConnection object is created, used and disposed:

  public void DALMethod()
  {
      SqlConnection connection = null;
      try
      {
          connection = new SqlConnection("XXXXXXXXXX");
          connection.Open();
          //implement the data access
      }
      catch (Exception exception)
      {
          //handle exception
      }
      finally
      {
          connection.Close();
          connection.Dispose();
      }
  }

In the above method the connection dispose is called explicitly in the finally block. In case there is an exception, then the catch block will be executed and after that the finally block will be executed to dispose the connection. So the connection is left in the memory until the finally block is executed. In the .NET Framework one of the basic guidelines is to release the resource when it is not being used anymore.

There is a better way of calling the dispose as shown in the below code:

  public void DALMethod()
  {
      using (SqlConnection connection = new SqlConnection("XXXXXXXXXX"))
      {
          connection.Open();
          //implement the data access
      }
  }

When you use the using block as shown above the dispose method on the object will be called as soon as the execution exits the block. This ensures that the SqlConnection resource is disposed and released at the earliest. You should also note that this will work on classes which implement the IDisposable interface.

Declaring Public Variables

This may sound simple, but it could really lead to the misuse of the public variables declared and could fetch unexpected use case of your class. Consider the below example:

  static void Main(string[] args)
  {
      
      MyAccount account = new MyAccount();
      //The caller is able to set the value which is unexpected
      account.AccountNumber = "YYYYYYYYYYYYYY";
  
      Console.ReadKey();    
  }
  
  public class MyAccount
  {
      public string AccountNumber;
  
      public MyAccount()
      {
          AccountNumber = "XXXXXXXXXXXXX";
      }
  

}

In the above example in the class MyAccount a public variable AccountNumber is declared and assigned in the constructor. It is desired that the AccountNumber should be read only and shouldn't be modified but the MyAccount class doesn't have any control over it.

A better way to declare a public variable like AccountNumber is to use property as shown below:

  public class MyAccount
  {
      private string _accountNumber;
      public string AccountNumber
      {
          get { return _accountNumber; }
      }
  
      public MyAccount()
      {
          _accountNumber = "XXXXXXXXXXXXX";
      }
  }

Here the MyAccount class has a good control over the AccountNumber. It is now read only and can't be set by the caller class.

Accessing Value from System.Data.DataTable

I frequently notice that most programmers access data from a datatable using column indexes as shown below:

  public class MyClass
  {
      public void MyMethod()
      {
  
          //GetData fetches data from the database using a SQL query
          DataTable dt = DataAccess.GetData();
          foreach (DataRow row in dt.Rows)
          {
              //Accessing data through column index
              int empId = Convert.ToInt32(row[0]);
          }
      }
  }

In the above scenario what if the column order in the SQL query fetching data changes, your application will break for sure. Always access the values through column names as shown below:

  public class MyClass
  {
      private const string COL_EMP_ID = "EmpId";
      public void MyMethod()
      {
  
          //GetData fetches data from the database using a SQL query
          DataTable dt = DataAccess.GetData();
          foreach (DataRow row in dt.Rows)
          {
              //Accessing data through column name
              int empId = Convert.ToInt32(row[COL_EMP_ID]);
          }
      }
  }

The code above won't break, even if the column order is changed. Use a constant variable to hold the column names at a single place so that even if the column name changes in the future then you will have to modify the code in only one place.

Conclusion

I hope you can learn from mistakes myself and other programmers have made in the past. These I believe are the 7 most frequent mistakes made by C# programmers. Please feel free to utilize the comments section to show case the conflicts and let me know what you think are common issues as well.

Related Articles





About the Author

V.N.S Arun

I work for an MNC in Bangalore, India. I am fond of writing articles, posting answers in forums and submitting tips in dotnet. To contact me please feel free to make use of the "Send Email" option next to the display name.

Comments

  • There are no comments yet. Be the first to comment!

Leave a Comment
  • Your email address will not be published. All fields are required.

Top White Papers and Webcasts

  • Instead of only managing projects organizations do need to manage value! "Doing the right things" and "doing things right" are the essential ingredients for successful software and systems delivery. Unfortunately, with distributed delivery spanning multiple disciplines, geographies and time zones, many organizations struggle with teams working in silos, broken lines of communication, lack of collaboration, inadequate traceability, and poor project visibility. This often results in organizations "doing the …

  • Download the Information Governance Survey Benchmark Report to gain insights that can help you further establish business value in your Records and Information Management (RIM) program and across your entire organization. Discover how your peers in the industry are dealing with this evolving information lifecycle management environment and uncover key insights such as: 87% of organizations surveyed have a RIM program in place 8% measure compliance 64% cannot get employees to "let go" of information for …

Most Popular Programming Stories

More for Developers

Latest Developer Headlines

RSS Feeds