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/).

Rules

AvoidCodeDuplicatedInSameClassRule

This rule checks for duplicated code in the same class. It ensures there aren't duplicated code in methods which belongs to the same class.

Bad example:

public class MyClass {
    private IList myList;
    
    public MyClass () {
        myList = new ArrayList ();
        myList.Add ("Foo");
        myList.Add ("Bar");
        myList.Add ("Baz");
    }
    
    public void MakeStuff () {
        foreach (string value in myList) {
            Console.WriteLine (value);
        }
        myList.Add ("FooReplied");
    }
    
    public void MakeMoreStuff () {
        foreach (string value in myList) {
            Console.WriteLine (value);
        }
        myList.Remove ("FooReplied");
    }
}

Good example:

public class MyClass {
    private IList myList;
    
    public MyClass () {
        myList = new ArrayList ();
        myList.Add ("Foo");
        myList.Add ("Bar");
        myList.Add ("Baz");
    }
    
    private void PrintValuesInList () {
        foreach (string value in myList) {
            Console.WriteLine (value);
        }
    }
    
    public void MakeStuff () {
        PrintValuesInList ();
        myList.Add ("FooReplied");
    }
    
    public void MakeMoreStuff () {
        PrintValuesInList ();
        myList.Remove ("FooReplied");
    }
}

AvoidCodeDuplicatedInSiblingClassesRule

This rule looks for code duplicated in sibling subclasses.

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:

  • Gtk.Bin::Build ()
  • Gtk.Window::Build ()
  • Gtk.Dialog::Build ()
  • 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....
}

AvoidMessageChainsRule

This rule avoids the Message Chain smell. This could cause some troubles to you, because your code is hardly coupled to the navigation structure.

Bad example:

public void Method (Person person)
{
    person.GetPhone ().GetAreaCode ().GetCountry ().Language.ToFrench ("Hello world");
}

Good example:

public void Method (Language language)
{
    language.ToFrench ("Hello world");
}

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 one subclass.
public abstract class AbstractClass {
    public abstract void MakeStuff ();
}
 
public class OverriderClass : AbstractClass {
    public override void MakeStuff ()
    {
    }
}

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 ()
    {
    }
}

AvoidSwitchStatementsRule

This rule avoids the Switch Statements smell. This kind of smell could lead your code to duplication, because the same switch could be repeated in various places of your program. Also, if you will need to do a little change, you should change every switch statement, the OO way is do it with polymorphism.

Bad example:

int balance = 0;
foreach (Movie movie in movies) {
    switch (movie.GetTypeCode ()) {
        case MovieType.OldMovie:
        balance += movie.DaysRented * movie.Price / 2;
        break
        case MovieType.ChildMovie:
        //its an special bargain !!
        balance += movie.Price;
        break;
        case MovieType.NewMovie:
        balance += (movie.DaysRented + 1) * movie.Price;
        break:
    }
}

Good example:

abstract class Movie {
    abstract int GetPrice ();
}
class OldMovie : Movie {
    public override int GetPrice () {
        return DaysRented * Price / 2;
    }
}
class ChildMovie : Movie {
    public override int GetPrice () {
        return movie.Price;
    }
}
class NewMovie : Movie {
    public override int GetPrice () {
        return (DaysRented + 1) * Price;
    }
}
 
int balance = 0;
foreach (Movie movie in movies)
balance += movie.GetPrice ()

Notes

  • This rule is available since Gendarme 2.4

Feedback

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