What is a" feature envy" code and why is it considered a code smell?

Question

This question on SO talks about correcting what the OP thought is feature envy code. Another example where I saw this nifty phrase being quoted is in a recently given answer here in programmers.SE. Although I did drop in a comment to that answer asking for the information I thought it would of general help to programmers following Q&A to understand what is meant by the term feature-envy. Please feel free to edit additional tags if you think appropriate.

Answer 1

Feature envy is a term used to describe a situation in which one object gets at the fields of another object in order to perform some sort of computation or make a decision, rather than asking the object to do the computation itself.

As a trivial example, consider a class representing a rectangle. The user of the rectangle may need to know its area. The programmer could expose width and height fields and then do the computation outside of the Rectangle class. Alternatively, Rectangle could keep the width and height fields private and provide a getArea method. This is arguably a better approach.

The problem with the first situation, and the reason it is considered a code smell, is because it breaks encapsulation.

As a rule of thumb, whenever you find yourself making extensive use of fields from another class to perform any sort of logic or computation, consider moving that logic to a method on the class itself.

Answer 2

There is a possible situation when it is OK to use another class/struct methods extensively - when your class/struct is a container for data. Usually there is a little you can do with this data without external context.

Such classes can still hold some internal logic but more often they are used as containers:

class YourUid {
 public:
  YourUid(int id_in_workplace_, int id_in_living_place_, DB* FBI_database, int id_in_FBI_database);
  bool IsInvalidWorker() const { return id_in_workplace == consts::invalid_id_in_workplace; }
  bool CanMessWith() const { return !FBI_database_.is_cool(id_in_FBI_database_); }
  int id_in_workplace;
  int id_in_living_place;
 private:
  int id_in_FBI_database_;
  const DB* FBI_database_;
};

@jhewlett in his answer refers to this article to prove that you should no use other class members extensively, but there is another code smells situation described there with advocates my example:

Long Parameter List. Limit the number of parameters you need in a given method, or use an object to combine the parameters.