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.

• gnat: possible duplicate of How large is ok for a Class? See also: Is it a code smell if an object knows a lot of its owner?

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.

• Doc Brown: +1, though your example is not realistic, since a useful Rectangle class would typically expose width and height fields either.
• Doc Brown: And though the breaking of encapsulation may a occur together with "Feature envirism", in most real-world examples I have seen so far it was probably not the case. It happens more in situations "hey, I need to compute some things just in the code using that object, and I am not sure if I can touch that object's implementation / if we should give the object the responsibility for that computation". And then one implements the calculation using fields which are already exposed (though a much cleaner implementation would be probably possible within the object).
• itsbruce: @DocBrown Imagine a rectangle drawn on the surface of a torus, cone or sphere. If my shape-drawing library produces objects which are capable of producing the correct results in such contexts, you'd be foolish not to leave them to calculate their own areas - in any context.
• Mvision: Let's say you had some DTO's that represented a rectangle, .. they get serialized to json and get used on the client. Calculating the area in that case will happen on the client side? Or would you advise to have an "area" property that gets calculated on the server side? (with a CalculateArea() function inside the DTO? or outside? In a lot of modern situations the whole thing isn't quite as clear. Where to put what, depends a LOT on the situation I guess :)
• Doc Brown: @OskarN.: it depends; sometimes the decision is clear, sometimes its a matter of taste, and most often its a matter of experience. In your article, there are good reasons why Scott Meyers writes "sometimes less is more", and that it took him years to understand when not to apply his "algorithm for deciding when to make f a member function".

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.

• gnat: how does this answer the question asked?
• Riga: @gnat The Q is about Why it is considered "code smell". jhewlett gives an A with some too general assumtions questioned in the comments. My answer is 2 cents to distinguish "code smell" from normal practice.