.Net Best Practices : Common Bugs Introduced By Refactoring, Carelessness, and Newbies

Question

What are the common bugs introduced by refactoring, carelessness, and newbies?

I would like to request the experienced programmers here to share their experience and list the bugs they used to introduce when they were inexperienced.

In your response, please write a headline mentioning the kind of bug in bold text, followed by few linebreaks, and then an explanation, cause of the bug, and finally the fix.

Answer 1

Manually written values instead of constants

Example:

public District GetDefaultDistrict() {
  return GetById(1);
}

public IList<Revenue> GetRevenues() {
  return GetByCodes(new [] { 8010, 8011, 8096 });
}

and thousands of use of 1, 8010, 8011 and 8096 in other places. Try to image if the default district now is 2 and 8011 moved to 8012.

Fix:

public District GetDefaultDistrict() {
  return GetById(Consts.DEFAULT_DISTRICT_ID);
}

public IList<Revenue> GetRevenues() {
  return GetByCodes(Consts.REVENUE_CODES);
}

and use this constants everywhere where you need to determine default district id and/or other static values.

Or even:

public IList<Revenue> GetRevenues() {
  var codes = GetRevenueCodes(); // get from db
  return GetByCodes(codes);
}

to get actual values from db. But this is just an example.

Answer 2

Property/String mismatch namely when using O/RM frameworks without linq:

    Dim crit As DetachedCriteria
    crit = DetachedCriteria.For(GetType(RemitLineItemEntity), "remit")

    If Not String.IsNullOrEmpty(LastName) Then
        If LastName.Contains("%") Then
            crit = crit.Add(New LikeExpression("LastName", LastName))
            countCrit = countCrit.Add(New LikeExpression("LastName", LastName))
        Else
            crit = crit.Add(Expression.Eq("LastName", LastName))
            countCrit = countCrit.Add(Expression.Eq("LastName", LastName))
        End If

    End If

Then changing the LastName property on the object to SurName, or such.

Fix

Only one I know of is to use a tool to do refactoring (which most do). They should catch the quoted field name as well, but usually use human verification on it (which leads to human error).

Answer 3

INotifyPropertyChanged bug introduced by refactoring, misspelling, case-mismatch etc.

Sometimes we change the name of some of our properties, and we forget to change the "string" we pass toOnPropertyChanged() method (especially when we call this method from somewhere else), or simply misspell it, including case-mismatch.

Something like this:

public string FirstName //earlier it was simply 'Name'
{
      get { return m_name; }
      set 
      { 
          m_name = value ;
          OnPropertyChanged("Name"); //still 'Name'. Bug!
         //or, OnPropertyChanged("FirstNane"); //changed, but misspelled. Bug!
      }
}

Fix

protected void OnPropertyChanged(string propertyName)
{
   //this raises exception if there is something wrong (only in debug mode!).
   RuntimeAssert.ValidatePropertyName(this, propertyName); 

   //your code here
}

Here is the implementation of RuntimeAssert class.

   public static class RuntimeAssert
    {
        private static Dictionary<Type, List<string>> ClassPropertyMap = new Dictionary<Type, List<string>>();
        private static List<string> GetProperties(Type type)
        {
            if (!ClassPropertyMap.ContainsKey(type))
            {
                PropertyInfo[] props = type.GetProperties(BindingFlags.NonPublic | BindingFlags.Public | BindingFlags.Instance | BindingFlags.Static);
                ClassPropertyMap.Add(type, new List<string>(props.Where(p => true).Select(p => p.Name)));
            }
            return ClassPropertyMap[type];
        }
        [Conditional("DEBUG")]
        public static void ValidatePropertyName(object instance, string propertyName)
        {
            ValidatePropertyName(instance.GetType(), propertyName);
        }
        [Conditional("DEBUG")]
        public static void ValidatePropertyName(Type type, String propertyName)
        {
            List<string> properties = RuntimeAssert.GetProperties(type);
            if (!properties.Contains(propertyName))
            {
                string message = String.Format("Property '{0}' not found in class '{1}'", propertyName, type.FullName);
                throw new PropertyNotFoundException(message);
            }
        }
    }

And finally, PropertyNotFoundException (it's used in above class).

public class PropertyNotFoundException : Exception
    {
        public PropertyNotFoundException(string message) : base(message) 
        {

        }
    }