A colleague and I have come to the conclusion that public classes named Impl are a code smell. Such classes are often a sign that not enough thought was put into naming the class. Let's look at an example.
Suppose I have a system that stores information about people in a MySQL database. I need a data access object (DAO) for the people, so I start by creating a PersonDao interface. Even though my system only has one way to store people, an interface is a good idea for the following reasons:
- I might want to introduce another implementation (say an LDAP directory) in the future.
- I want to write unit tests for classes that depend on a PersonDao and an interface makes it easier to mock the dependency. You do write unit tests, right?
So what do I name the class that implements the PersonDao interface and uses a MySQL database? I only have one implementation class, so why not name it PersonDaoImpl?
That's a bad choice for a few reasons:
- It doesn't accurately describe what the class is.
- It leads to inconsistencies when further implementation classes are added in the future. For example, suppose a few years after release we discover that some of our customers would rather have their people stored in an LDAP directory. We can't name the LDAP implementation class PersonDaoImpl because that name is already chosen so we use LdapPersonDao. Now we're left with similar classes with dissimilar names.
A better choice is to name the original MySQL implementation MySqlPersonDao and avoid these problems.
Spend a little time to name your classes accurately. If you're having trouble with that, your classes are probably doing too much and you need to refactor.