I was looking at some code and a simple rule just occurred to me. If you’re creating an object oriented system, any time you start writing a method that accepts an instance of a class that you created as an argument, you should ask yourself whether that method should really be a member of the object being passed in as an argument.
I’d also recommend that any time you’re asking a question about an object (in a loop condition or an if statement, for example), it’s worth considering whether that logic should be encapsulated as a method of the object as well.
For example, you might have code like this:
if (person.getAge() > 18 && person.getCountryOfBirth().equals(Country.USA)) {
// allow person to vote
}
It should probably be written like this:
if (person.isEligibleToVote()) {
// allow person to vote
}
The second example is easier to read and makes it easier to avoid keeping the voter eligibility conditions in more than one place. For example, I know that someone is going to file a bug noting that my expression doesn’t account for naturalized citizens. Keeping that logic in its own method will make it easier to fix that bug.
January 30, 2009 at 9:36 am
This is a good rule for classes that are not part of a public API and that have a very specific purpose which will not change over time. For larger projects, the class tends to get polluted with many of these methods.
I’ve found the Specification pattern described in Domain Driven Design to be especially useful in this kind of situation. There are many ways to implement it depending on the simplicity/scalability tradeoff, but the basic idea is that you externalize testing of whether an instance meets some criteria to a helper class.
Specification isEligibleToVote(); if (isEligibleToVote(person)) { // allow person to vote }January 30, 2009 at 11:30 am
yeah, if it is an otherwise static class (or essentially static, whether explicitly declared static or not).
i’d say generally look for the object referenced most frequently in the method’s implementation–make it an instance method of that object’s class, i.e. make that object be “this”.
January 30, 2009 at 5:37 pm
<p>Too bad 18 year olds can’t vote according to your logic!</p>
January 31, 2009 at 10:11 am
In response to Jake’s comment, I do agree that in big projects classes tend to acquire a ton of these sorts of methods. I’m not sure it’s pollution, though. For stuff like this, the question is, where are other developers (or you, in the future) most likely to find this kind of code. If people can safely assume that if the method has been written, it’s a member of the class it affects, it makes it less likely that the current implementation will be reimplemented because the developer never really bothered to find out whether someone had already done it.