Today I was reading an answer to a question (http://bit.ly/FkgKB) about naming the implementation classes. People in these days are using Impl suffix to their implementation classes which implements a certain interface. For instance, if your interface name is Item, your implementation class name would be ItemImpl. Honestly I hate this bullshit naming convention. And I’ll forever blame the guy who invented such a crappy naming. Actually I first saw this naming convention in the IBM Developerworks articles. I guess it is some sort of IBM crap (I never got along with any of IBM related thing so far though). Anyways, let me tell you why it’s crappy:
First of all, it shows me that most likely you can’t come up with any good name to your class that could represent the responsibility of it. It basically smells like hell. Second which name are you going to give to the next implementation of same class? Of course you can’t find a good name for that too. Most likely scenario will be like this: Item, ItemImpl, CatalogItemImpl and so on.. Lots of duplication. And third, by looking at the class name, you can’t tell what this implementation does actually. Please consider Item and ItemImpl. As you may easily figure, it’s very hard to tell what ItemImpl does in the context without looking at the code itself.
Another weird idea to me is putting I prefix before interface names. I can’t get it too. Are you doing this just to separate your interfaces from classes? Excuse me, which IDE is not showing whether it is interface or not? Look at these: IPerson, PersonImpl.. What happened to my Person domain model? It’s just ugly.
So what I think? I think that if you’re placing an interface, implementation class name should tell you what does that class do in that context. For instance, if you have a XMLParser interface, your implementations should be XMLFileParser, XMLInputStreamParser etc.. If you cannot find a decent name for your class, maybe your should consider your design again. Maybe it’s not fitting well in the context. I don’t like creating domain object names as interfaces either. It just doesn’t make sense to me. Look at this: Customer (interface), CustomerImpl is implementation. Why don’t you create a Customer implementation which implements Identity? Or instead of LineItem, LineItemImpl couple, maybe you should consider naming them like LineItem implements Listable, Sortable? Basically what I’m saying is actually your interface design should be accurate to represent the context. Because all of your APIs will interact with these domain models.
When you’re applying Law of Demeter, you shouldn’t forget names as well as class fields and methods so that we can understand which class is actually doing what.
There is a really nice explanation of this naming problem. I’ll quote it directly from mockobjects.com:
Sometimes we see code with classes that are named by adding “Impl ” to the single interface they implement. This is better than leaving the class name unchanged and prefixing an “I ” to the interface, but not much. A name like FooImpl is duplication, it says exactly the same as implements Foo , which is a code smell. We would not tolerate such obvious duplication elsewhere in our code, so we ought to refactor it away.
It might just be a naming problem. There’s always something specific about an implementation that can be included in the class name: it uses a bounded collection, it communicates over HTTP, it uses a database for persistence, and so on. A bridging class is even easier to name, since it will belong in one domain but implement interfaces in another: for example, SnipersTableModel (a user interface class) implements SniperListener and PortfolioListener (from the application core).
If there really isn’t a good implementation name, it might mean that the interface is poorly named or designed. Perhaps it’s unfocussed because it has too many responsibilties, or it’s named after its implementation rather than its role in the client, or (just occasionally) that it’s a value not an object—this discrepancy sometimes turns up when writing unit tests
Agree with you, but I have a habit to use “Impl” – Guilty :-). I think that a lot of people create implementation and interface together, or implementation first, so interface name usually describes first implementation – this is probably the root of the problem.
July 18, 2009, 8:54 PMI really hate “I” like interface convention, I think it comes from C# (Microsoft).
@andrew, yeah that’s another problem too. If you have only one implementation for that particular interface and it’s hanging there 3 years, then YAGNI says maybe you don’t need an interface there :) A bit of the problem comes from Spring DI mechanism though. But that’s another story..
July 18, 2009, 9:18 PMIt costs additional efforts to supply implementation with good descriptive name. People are lazy, it’s our nature.
From other side if there is interface CakeCooker then we probably can name implementations like FastCakeCooker, HTTPCakeCooker, MockCakeCooker but it doesn’t resolve duplication problem.
BTW often it’s harder to get working software then to figure out naming conventions
July 18, 2009, 10:57 PM@pavel, my point about duplication was not about the Cooker part. It’s about the Impl part. It’s a suffix that doesn’t mean anything. It’s like this in your example: CakeCooker, FactCakeCookerClass, HTTPCakeCookerClass, MockCakeCookerClass etc.. Of course it’s class, we know that. Why repeat again and again..
Then again, I agree about our nature. That’s biggest cumbersome we have actually :)
July 19, 2009, 4:58 PMYou make a good point in your first comment. If you are forced by framework to violate YAGNI and create an interface due to spring, which pushes you to make a decision of creating IPerson or PersonImpl, maybe its time to question the usage of the framework.
July 19, 2009, 5:13 PM@dan Spring issue is really weird. Most of its users are using Spring just for DI, nothing more.. I don’t know, IMHO Spring is way big for a DI mechanism. I remember I read an article about how Google Guice is 100 times faster than Spring in DI.. Anyways..
July 19, 2009, 11:57 PMOne more issue I see here….What if we have and Cooker (interface) AbstractCooker (abstract class), A BaseCooker (Base class to / can be extended by all cookers) and FastCooker, SlowCooker ? Isn’t Base and Abstract necessary ?
Same way if we have Cooker enum, It would better spelled as CookerType or CookerEnum ? CookerEnum says its enum and easy to identify at first glance ?
I agree with case of context. FastCookerImpl and SlowCookerImpl is not needed. But What if we Have FastCooker interface and SlowCooker Interface and implementation as FastRealCooker and FastFakeCooker ? Isn’t it easier to find “Intf” and “Impl” clearly ?
January 22, 2010, 12:52 AM@jigar, I see your point. But when you’re naming the classes, I believe you should be talking in ubiquitous language. Your design details come after this perspective. In your case I don’t think putting base or abstract would make any difference since after a while you’ll find everyone is naming their classes BaseXXX, AbstractXXX. And then you’ll notice that (in the project I work right now) Abstract classes extending from other AbstractBase classes :) LOL. It’s just crazy.
Anyways, I’d rename your cookers like this if it makes sense to you: Cooker (your base or abstract), FastCooker (fast one), SlowCooker (slow one). If you really need to have base and abstract at the same time, I think you should just look at your design, maybe something shouldn’t be there. Then again I don’t know your context. For interface vs Impl difference that you say, FastCooker cannot be an interface to me. Being Fast and can Cook are 2 different concerns. You’re trying to merge them. That’s why you think an interface (FastCooker) and an implementation (FastRealCooker) will be needed. I’d say, make CanCook, IsFast and IsSlow interfaces and implement FastCooker with CanCook and IsFast together. You can do better encapsulation with this way.
As I said, all these are just from my perspective. Maybe people think I’m bullshitting. But when I see a name contains Can, Is, Able, etc I know for sure it’s interface. Beside, any stupid IDE can tell you if it’s an interface or implementation.
January 22, 2010, 10:20 AMdisagree with u guys.it depends on your business logic.if u need a ReservationService interface and your company is just a restaurant or somethng like that, u cant call its implementations as RestaurantReservationService or RichCustomerReservationService..that would be silly.of course ide can tell us which one is a interface or a concrete impl. so u recommend us to add a impl. package to our projects with exactly the same class names? like com.foo.service.ReservationService and com.foo.service.impl.ReservationService.. =)
February 16, 2010, 4:33 PM@beko I guess u lost the point somewhere. If you do have 2 possible implementations of that ReservationService, u should make implementation class names meaningful. In ur example I’d have: com.foo.service.ReservationService (interface), com.foo.service.impl.OpenTableReservationService, com.foo.service.impl.YelpReservationService kinda. If you don’t have multiple implementations of that interface, YAGNI should stop u to have an interface there, don’t u think?
When u don’t give meaningful names to your classes, then as u pointed too, u end up having stupid names like RestaurantReservationService. If you can’t give a meaningful name to ur classes, I’d say ur class is either not supposed to be there, or it has more than one reason to change. I hope this clarifies a bit.
February 16, 2010, 7:37 PM