Gendarme.Rules.Smells

Gendarme's refactoring suggestion rules are located in the Gendarme.Rules.Smells.dll assembly. Latest sources are available from anonymous SVN (http://anonsvn.mono-project.com/viewcvs/trunk/mono-tools/gendarme/rules/Gendarme.Rules.Smells/) (tarball (http://anonsvn.mono-project.com/viewcvs/trunk/mono-tools/gendarme/rules/Gendarme.Rules.Smells.tar.gz?view=tar)).

Rules

AvoidCodeDuplicatedInSameClassRule

This rule looks for code duplicated in methods that belongs to the same class. You can use it as a detector for the Code Duplicated smell.

Bad example: 

public class MyClass {
 
    ArrayList list = new ArrayList ();
 
    public void PrintAndAddANewValue ()
    {
        foreach (string value in list) {
            Console.WriteLine (value);
        }
        list.Add ("foo");
    }     
 
    public void PrintAndRemoveAValue ()
    {
        foreach (string value in list) {
           Console.WriteLine (value);
        }   
        list.Remove ("foo");
    } 
}

Good example: 

public class MyClass {
 
    ArrayList list = new ArrayList ();
 
    public void PrintAndAddANewValue ()
    {
        PrintValues ();
        list.Add ("foo");
    }    
 
    public void PrintValues ()
    {
        foreach (string item in list) {
           Console.WriteLine (item);
        }
    }     
 
    public void PrintAndRemoveAValue ()
    {
        PrintValues ();
        list.Remove ("foo");
    } 
}


AvoidCodeDuplicatedInSiblingClassesRule

This rule is used for look for code duplicated in sibling classes. You can use it as a detector for Code Duplicated smell.

Bad example: 

public class BaseClassWithCodeDuplicated {
   protected IList list;
}
 
public class OverriderClassWithCodeDuplicated : BaseClassWithCodeDuplicated {
 
   public void CodeDuplicated ()
   {
      foreach (int i in list)
         Console.WriteLine (i);
      list.Add (1);
   }
}
 
public class OtherOverriderWithCodeDuplicated : BaseClassWithCodeDuplicated {
 
   public void OtherMethod ()
   {
      foreach (int i in list)
         Console.WriteLine (i);
      list.Remove (1);
   }
}

Good example: 

public class BaseClassWithoutCodeDuplicated {
   protected IList list;
 
   protected void PrintValuesInList ()
   {
      foreach (int i in list)
         Console.WriteLine (i);
   }
}
 
public class OverriderClassWithoutCodeDuplicated : BaseClassWithoutCodeDuplicated {
 
   public void SomeCode ()
   {
      PrintValuesInList ();
      list.Add (1);
   }
}
 
public class OtherOverriderWithoutCodeDuplicated : BaseClassWithoutCodeDuplicated {
 
   public void MoreCode ()
   {
      PrintValuesInList ();
      list.Remove (1);
   }
}


AvoidLargeClassesRule

This rule allows developers to measure the classes size. When a class is trying to doing a lot of work, then you probabily have the Large Class smell.

The rule counts all fields and if exists common prefixes in fields it could be a point for extract a new class too. It's quite hard determine when a class is doing a lot of work, then in following versions you will can select a maximum number of fields, by default this amount is 25.

Bad example: 

public class MyClass {
    int x, x1, x2, x3;
    string s, s1, s2, s3;
    DateTime bar, bar1, bar2;
    string[] strings, strings1; 
}


Good example: 

public class MyClass {
    int x;
    string s;
    DateTime bar; 
}


AvoidLongMethodsRule

This rule allows developers to measure the methods size. The short sized methods allows you to maintain your code better, if you have long sized methods perhaps you will have the Long Method smell.

The rule will skip some well known methods, because they are autogenerated:

  • Type Gtk.Bin -> Build
  • Type Gtk.Window -> Build
  • Type Gtk.Dialog -> Build
  • Type System.Windows.Forms -> InitializeComponents

This will work for classes that extends those types.

If debugging symbols (e.g. Mono .mdb or MS .pdb) are available then rule will compute the number of logical source line of code (SLOC). This number represent the lines where 'SequencePoint' are present in the code. By default the maximum SLOC is defined to 40 lines.

Otherwise the rule falls back into an IL-SLOC approximation. It's quite hard too determine how many SLOC exists based on IL (e.g. LINQ). The metric being used is based on a screen (1024 x 768) full of source code where the number of IL instructions were counted. By default the maximum number of IL instructions is defined to 165.

Bad example: 

public void LongMethod ()
{
   Console.WriteLine ("I'm writting a test, and I will fill a screen with some useless code");
   IList list = new ArrayList ();
   list.Add ("Foo");
   list.Add (4);
   list.Add (6);
 
   IEnumerator listEnumerator = list.GetEnumerator ();
   while (listEnumerator.MoveNext ())
      Console.WriteLine (listEnumerator.Current);
 
   try {
      list.Add ("Bar");
      list.Add ('a');
   }
   catch (NotSupportedException exception) {
      Console.WriteLine (exception.Message);
      Console.WriteLine (exception);
   }
 
   foreach (object value in list) {
      Console.Write (value);
      Console.Write (Environment.NewLine);
   }
            
   int x = 0;
   for (int i = 0; i < 100; i++)
      x++;
   Console.WriteLine (x);
    
   string useless = "Useless String";
   if (useless.Equals ("Other useless")) {
      useless = String.Empty;
      Console.WriteLine ("Other useless string");
   }
            
   useless = String.Concat (useless," 1");
   for (int j = 0; j < useless.Length; j++) {
      if (useless[j] == 'u')
         Console.WriteLine ("I have detected an u char");
      else
         Console.WriteLine ("I have detected an useless char");
   }
            
   try {
      foreach (string environmentVariable in Environment.GetEnvironmentVariables ().Keys)
         Console.WriteLine (environmentVariable);
   }
   catch (System.Security.SecurityException exception) {
      Console.WriteLine (exception.Message);
      Console.WriteLine (exception);
   }
 
   Console.WriteLine ("I will add more useless code !!");
   try {
      if (!(File.Exists ("foo.txt"))) {
         File.Create ("foo.txt");    
         File.Delete ("foo.txt");
      }
   }
   catch (IOException exception) {
      Console.WriteLine (exception.Message);
      Console.WriteLine (exception);
   }
}

Good example: 

public void ShortMethod ()
{
   try {
      foreach (string environmentVariable in Environment.GetEnvironmentVariables ().Keys)
         Console.WriteLine (environmentVariable);
   }
   catch (System.Security.SecurityException exception) {
      Console.WriteLine (exception.Message);
      Console.WriteLine (exception);
   }
}

AvoidLongParameterListsRule

This rule allows developers to measure the parameter list size in a method. If you have methods with a lot of parameters, perhaps you have a Long Parameter List smell.

This rule counts the method parameters, and compare against a maximum value. If you have an overloaded method, then the rule will get the shortest overload and compare the shortest overload against the maximum value.

Other time, it's quite hard determine a long parameter list. By default, a methods with 6 or more arguments will be notified.

Bad example: 

public void MethodWithLongParameterList (int x, char c, object obj, bool j, string f,
                                         float z, double u, short s, int v, string[] array)
{
   // Method body ... 
}


Good example: 

public void MethodWithoutLongParameterList (int x, object obj)
{
    // Method body.... 
}


AvoidSpeculativeGeneralityRule

This rule allows developers to avoid the Speculative Generality smell. Be carefull if you are developing a new framework or a new library, because this rule only inspect the assembly, then if you provide an abstract base class for extend by thrid party people, then the rule can warn you. You can ignore the message in this special case.

We can detect this kind of smell looking for some points:

  • Abstract classes without responsability.
  • Unnecesary delegation.
  • Unused parameters.

Bad example: An abstract class with only a subclass. 

    public abstract class AbstractClass {
        public abstract void MakeStuff ();
    }
 
    public class OverriderClass : AbstractClass {
        public override void MakeStuff ()
        {
        
        }
    }

Bad example: If you use Telephone class only in one client, perhaps you don't need this kind of delegation. 

     public class Person {
        int age;
        string name;
        Telephone phone;
    }
 
    public class Telephone {
        int areaCode;
        int phone;
    }

Good example: 

public abstract class OtherAbstractClass{
   public abstract void MakeStuff ();
}
 
public class OtherOverriderClass : OtherAbstractClass {
   public override void MakeStuff ()
   {
   }
}
 
public class YetAnotherOverriderClass : OtherAbstractClass {
   public override void MakeStuff ()
   {
   }
}

Feedback

Please report any documentation errors, typos or suggestions to the Gendarme Google Group (http://groups.google.com/group/gendarme). Thanks!