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.
Feature Envy Code – What It Is and Why It’s a Code Smell
clean codecode smellcode-reviewsrefactoring
Related Solutions
By using this mocking mechanism, you don't just affect your @client. You affect all matching objects within that test. Here, this isn't much of a problem since only one such object happens to exist, but consider what would happen if you were to affect a more fundamental type like strings or numbers that is used throughout your program.
The reliance on this mocking mechanism is a strong indicator that your design isn't testable in itself – in particular, the fixed dependency on Twilio::REST::Client is problematic. By using a dependency injection mechanism such as constructor injection, we can write the same test but with much less magic.
Here's pseudocode to illustrate the concept:
class SmsSender
def initialise(message, sender)
@message = message
@sender = sender || make_default_sender()
end
def send_to(phone)
return false unless validate(phone)
@sender({
from: '123',
to: format_phone(phone),
message: @message,
})
end
end
def make_default_sender():
client = Twilio::REST::Client.new(SECRET)
return do |message|
client.send(message)
end
end
In the test, we can now easily check that we're sending the correct message:
expected_message = ...
checker_was_called = false
def checking_sender(message)
checker_was_called = true
assert message == expected_message
# you could still send the message here if you want to.
end
SmsSender.new('...', checking_sender).send_to('...')
assert checker_was_called
Of course this is also much more code than your current test, and dependency injection always introduces some fragility into the system – you really need integration tests to make sure all dependencies are wired up correctly for production/deployment. It can therefore make a lot of sense to keep your current approach, as long as you're aware of the trade-offs.
There is absolutely nothing wrong with having pure data objects. The author of the piece quite frankly doesn't know what he's talking about.
Such thinking stems from an old, failed, idea that "true OO" is the best way to program and that "true OO" is all about "rich data models" where one mixes data and functionality.
Reality has shown us that actually the opposite is true, especially in this world of multi-threaded solutions. Pure functions, combined with immutable data-objects, is a demonstrably better way to code.
Best Answer
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
widthandheightfields and then do the computation outside of theRectangleclass. Alternatively,Rectanglecould keep thewidthandheightfields private and provide agetAreamethod. 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.