What Is Feature Envy?

Photo by James Jeremy Beckers on Unsplash

Today’s topic is ‘Feature Envy’, a code smell and an indicator of deeper problems with the software. Feature Envy means that one class is ‘envious’ of, and contains, behaviour that rightly belongs to another. The code is in the wrong place: One type is doing too much and the other, not enough.

In extreme cases, we can end up with God-classes that fail to delegate work and do everything themselves. In my experience, much procedural code written in object-oriented languages is plagued by Feature Envy. 

By removing instances of Feature Envy, we get better encapsulation, code reuse and balancing of responsibilities between classes. 

Let’s look a couple of examples of Feature Envy: 

Example 1:

  private async Task Validate(CustomerRegistration registration)
  {
     if (registration == null)
        throw new MissingCustomerRegistration();

     if (string.IsNullOrWhiteSpace(registration.FirstName))
        throw new MissingFirstName();
     if (string.IsNullOrWhiteSpace(registration.LastName))
        throw new MissingLastName();
     if (string.IsNullOrWhiteSpace(registration.EmailAddress))
        throw new MissingEmailAddress();

     var existCust = await Repository.GetCustomer(registration.EmailAddress);
     if (existCust != null)
        throw new DuplicateCustomerEmailAddress(registration.EmailAddress);
  }

Validate() is doing more than it should. Can you identify the code that we could move to a better home?

That’s right; it’s the detailed validation of the CustomerRegistration parameter. The code is performing multiple validation checks that would make more sense in the CustomerRegistration class. Let’s move the code over into a new public Validate() method on CustomerRegistration:

  public class CustomerRegistration
  {
     // ...
     
     public void Validate()
     {
        if (string.IsNullOrWhiteSpace(FirstName))
           throw new MissingFirstName();
        if (string.IsNullOrWhiteSpace(LastName))
           throw new MissingLastName();
        if (string.IsNullOrWhiteSpace(EmailAddress))
           throw new MissingEmailAddress();
     }
  }

Now we are in a position to replace the detailed validation in the original Validate() method with a call to CustomerRegistration.Validate():

  private async Task Validate(CustomerRegistration registration)
  {
     if (registration == null)
        throw new MissingCustomerRegistration();
     registration.Validate();
     var existCust = await Repository.GetCustomer(registration.EmailAddress);
     if (existCust != null)
        throw new DuplicateCustomerEmailAddress(registration.EmailAddress);
  }

Much simpler and shorter than before! 

Example 2:

A private helper method IsPublicSchool() is defined as: 

  private bool IsPublicSchool(School school)
  {
     return school.Type == 2;
  }

, and called several times in conditional statements:

  if (IsPublicSchool(school))

The Type property is part of the School class. So why do we determine outside of class School whether a school is public or not? That does not feel right. Let’s fix this by adding an IsPublic property to School:

  public bool IsPublic => Type == 2;

The conditional statements simplify to:

  if (school.IsPublic)

Much better! 

How do we identify Feature Envy?

I think a decent indicator is when one class does an extended amount of work with the instance of another class. The registration validation in our first example was like that.

In our second example, we were building behaviour in another class that we could entirely determine inside the School type.

Whenever I remove some Feature Envy, the code feels more natural and balanced. It’s great.