In the previous blog post we have seen how to detect potential God Classes with NDepend. In this article we’ll see how to detect methods that suffer from Feature Envy.

Feature Envy Detection Strategy

The feature envy code smell refers to methods that access data from other sources, rather than their own. Object-Oriented Metrics in Practice, by Michele Lanza and Radu Marinescu, proposes the following detection strategy for Feature Envy:

(ATFD > Few) AND (LAA < One Third) AND (FDP <= Few)

This detection strategy uses three metrics:

• ATFD – Access To Foreign Data – to measure how many foreign attributes the method is using
• LAA – Locality of Attribute Accesses – to measure if the method uses more attributes from other classes than its own
• FDP – Foreign Data Providers – to measure the number of classes that the foreign attributes belong to

This detection strategy uses two types of thresholds:

• ATFD and FDP use Generally-Accepted Meaning Thresholds. FEW is defined between 2 and 5.
• LAA uses a Common Fraction Threshold. One Third is 0.33.

Metrics Definitions

Let’s go over the definitions for the used metrics and how to implement them with NDepend. For a more detailed definition, be sure to check Appendix A.2 of Object-Oriented Metrics in Practice. If you’re not familiar with CQLinq, check out the NDepend documentation or my blog post on how to query your code base. ATFD has also been defined in my previous blog post. I’ve made some changes in the meantime, so we’ll redefine it here.

ATFD – Access To Foreign Data

This metric measures the number of attributes from unrelated classes that are accessed directly or through accessor methods. For the Feature Envy code smell, we use the ATFD metric for a method.

// <Name>ATFD</Name>
// ** Helper Functions **
let isProperty = new Func<ICodeElement, bool>(member =>
 member.IsMethod &&
 (member.AsMethod.IsPropertyGetter || member.AsMethod.IsPropertySetter))

let isPropertyOrField = new Func<ICodeElement, bool>(member =>
 isProperty(member) || member.IsField)

let classHierarchyFor = new Func<IType, HashSet<IType>>(t =>
 t.BaseClasses.Append(t).ToHashSet())

// ** Metric Functions **
let foreignDataFor = new Func<IMethod, IEnumerable<IMember>>(m =>
 m.MembersUsed.Where(member=> 
   !classHierarchyFor(m.ParentType).Contains(member.ParentType) &&
   isPropertyOrField(member) && 
   // Ignore Nullable
   !member.ParentType.NameLike("Nullable<T>") &&
   // Ignore Async related classes
   !member.ParentNamespace.NameLike("System.Runtime.CompilerServices")))

let atfdForMethod = new Func<IMethod, int>(m =>
 foreignDataFor(m).Count())

// ** Sample Usage **
from m in JustMyCode.Methods
let atfd = atfdForMethod(m)
orderby atfd descending
select new { m, atfd }

Also, if you look at lines 17-21 you’ll notice that we ignore Nullables and types from the System.Runtime.CompilerServices namespace. This is because I think these are false positives. Nullable<T> should be ignored, since we already consider T. Also, the System.Runtime.CompilerServices namespace contains compiler generated code (e.g. code generated by async/await usage), so I decided to ignore it.

LAA – Locality of Attribute Accesses

This metric measure the relative number of local attributes versus foreign attributes. It’s computed by dividing the number of attributes from the method’s definition class by the total number of attributes accessed. Since properties are used a lot in C# code bases (even private properties), I’ve decided to also include them when computing the number of attributes from the definition class. This is the beauty of custom metrics: you can tailor the metrics for your own context.

// <Name>LAA</Name>
// ** Helper Functions **
let isProperty = new Func<ICodeElement, bool>(member =>
 member.IsMethod &&
 (member.AsMethod.IsPropertyGetter || member.AsMethod.IsPropertySetter))

let isPropertyOrField = new Func<ICodeElement, bool>(member =>
 isProperty(member) || member.IsField)

let allMembersUsedFor = new Func<IMethod, IEnumerable<IMember>>(m =>
 m.MembersUsed.Where(member => isPropertyOrField(member)))

let ownMembersUsedFor = new Func<IMethod, IEnumerable<IMember>>(m =>
 allMembersUsedFor(m).Where(member => member.ParentType == m.ParentType))

// ** Metric Functions **
let laaFor = new Func<IMethod, double>(m =>
 (double) ownMembersUsedFor(m).Count() / allMembersUsedFor(m).Count())

// ** Sample Usage **
from t in JustMyCode.Methods
let laa = laaFor(t)
orderby laa
select new { t, laa }

FDP – Foreign Data Providers

This metric measure the number of classes in which the foreign attributes are defined. Since this is related to ATFD, it reuses many of the helper functions:

// <Name>FDP</Name>
// ** Helper Functions **
let isProperty = new Func<ICodeElement, bool>(member =>
 member.IsMethod &&
 (member.AsMethod.IsPropertyGetter || member.AsMethod.IsPropertySetter))

let isPropertyOrField = new Func<ICodeElement, bool>(member =>
 isProperty(member) || member.IsField)

let classHierarchyFor = new Func<IType, HashSet<IType>>(t =>
 t.BaseClasses.Append(t).ToHashSet())

// ** Metric Functions **
let foreignDataFor = new Func<IMethod, IEnumerable<IMember>>(m =>
   m.MembersUsed.Where(member=> 
   !classHierarchyFor(m.ParentType).Contains(member.ParentType) &&
   isPropertyOrField(member) && 
   // Ignore Nullable
   !member.ParentType.NameLike("Nullable<T>") &&
   // Ignore Async related classes
   !member.ParentNamespace.NameLike("System.Runtime.CompilerServices")))

let foreignDataProvidersFor = new Func<IMethod, IEnumerable<IType>>(m =>
 foreignDataFor(m).Select(member => member.ParentType).ToHashSet())

let fdpFor = new Func<IMethod, int>(m =>
 foreignDataProvidersFor(m).Count())

// ** Sample Usage **
from m in JustMyCode.Methods
let fdp = fdpFor(m)
orderby fdp descending
select new { m, fdp }

Putting it all together

Now that we know how to compute each of the required metrics, let’s see how the detection strategy looks like:

// <Name>Feature Envy</Name>
warnif count > 0
// *** ATFD & FDP ***
// ** Helper Functions **
let isProperty = new Func<ICodeElement, bool>(member => 
 member.IsMethod && 
 (member.AsMethod.IsPropertyGetter || member.AsMethod.IsPropertySetter)) 

let isPropertyOrField = new Func<ICodeElement, bool>(member => 
 isProperty(member) || member.IsField)

let classHierarchyFor = new Func<IType, HashSet<IType>>(t => 
 t.BaseClasses.Append(t).ToHashSet())

// ** Metric Functions **
let foreignDataFor = new Func<IMethod, IEnumerable<IMember>>(m => 
 m.MembersUsed.Where(member=> 
    !classHierarchyFor(m.ParentType).Contains(member.ParentType) &&
    isPropertyOrField(member) && 
    // Ignore Nullable
    !member.ParentType.NameLike("Nullable<T>") &&
    // Ignore Async related classes
    !member.ParentNamespace.NameLike("System.Runtime.CompilerServices")))

let foreignDataProvidersFor = new Func<IMethod, IEnumerable<IType>>(m => 
 foreignDataFor(m).Select(member => member.ParentType).ToHashSet())

let atfdForMethod = new Func<IMethod, int>(m => 
 foreignDataFor(m).Count())

let fdpFor = new Func<IMethod, int>(m =>
 foreignDataProvidersFor(m).Count())

// *** LAA ***
// ** Helper Functions **
let allMembersUsedFor = new Func<IMethod, IEnumerable<IMember>>(m => 
 m.MembersUsed.Where(member => isPropertyOrField(member)))

let ownMembersUsedFor = new Func<IMethod, IEnumerable<IMember>>(m =>
 allMembersUsedFor(m).Where(member => member.ParentType == m.ParentType))

// ** Metric Functions **
let laaFor = new Func<IMethod, double>(m => 
 (double) ownMembersUsedFor(m).Count() / allMembersUsedFor(m).Count())

// ** Thresholds **
let OneThird = 0.33

// ** Detection Strategy **
from m in JustMyCode.Methods
let atfd = atfdForMethod(m)
let laa = laaFor(m)
let foreignDataProviders = foreignDataProvidersFor(m)
let foreignData = foreignDataFor(m)
let fdp = foreignDataProviders.Count()

where 
 // Method uses directly more than a few attributes from other classes
 atfd > 5 &&
 // Method uses far more attributes of other classes than its own
 laa < OneThird &&
 // The used foreign attributes belong to very few other classes
 fdp <= 3

select new { m, atfd, laa, fdp, foreignDataProviders, foreignData }

Conclusion

Implementing this detection strategy was quite easy. The only thing that is bothering me is code duplication. We’ll reuse some of the metrics in the detection strategies of multiple code smells (e.g. God Class and Feature Envy both rely on ATFD). How can we reuse some of our functions? Or maybe compute the metrics only once? I think that this would be possible by using NDepend API, but I prefer using CQLinq. If you have any ideas for improvement, please leave a comment.