I'll try to give an answer against such usage, after having seen and worked on this in one of my projects as well.
Code readability
First of all, consider that code readability is important and this practice is counter to that. Someone reads a piece of code and let's just say it has a function doSomething(Employee e)
. This is no longer readable, because due to 10 different Employee
classes existing in different packages you will have to first resort to the import declaration to find out what your input really is.
This, however, is a high-level view and we often have seemingly random name clashes, which no one cares about or even finds, because the meaning can be derived from the remaining code and which package you are in. So one could even argue that, locally, there is no problem, because of course if you see Employee
within an hr
package you have to know that we're talking about an HR view of the employee.
Things break apart as soon as you leave these packages though. Once you are working in a different module/package/etc and need to work with an employee, you're already sacrificing readability if you do not fully qualify its type. Additionally, having 10 different Employee
classes means that your IDE's auto-completion will no longer work and you have to manually choose from the type of employee.
Code duplication
Due to the very nature of each of these classes being related to each other your code is bound to deteriorate due to lots of duplication. In most cases, you will have something like an employee's name or some identification number, which each of the classes has to implement. Even though each class adds its own sort of view, if they do not share the underlying employee data, then you will end up with huge masses of useless, but costly, code.
Code complexity
What can be so complex you may ask? After all, each of the classes can keep it as simple as it wants to. What truly becomes a problem is how you propagate changes. In a reasonably feature-rich software you may be able to change employee data - and you want to reflect that everywhere. Say that one lady just married and you have to rename her from X
to Y
due to that. Hard enough to do this right all over the place, but even harder when you have all of these distinct classes. Depending on your other design choices, this may easily mean that each of the classes has to implement its own sort of listener or such to be notified of changes - which basically translates to a factor being applied to the number of classes you have to deal with. And more code duplication of course, and less code readability, .. Factors like this may be ignorable in complexity analysis, but they sure are annoying when applied to the size of your code base.
Code communication
Apart from the above problems with code complexity, which are also related to design choices, you are also reducing your higher-level design clarity and lose the ability to properly communicate with domain experts. When you discuss architecture, designs, or requirements you are no longer free to make simple statements like given an employee, you can do ...
. A developer will no longer know what employee
really means at that point. Although a domain expert will know it of course. We all do. sort of. But in terms of the software it's not so easy to communicate anymore.
How to get rid of the problem
After becoming aware of this and if your team agrees that it is a problem big enough to tackle, then you will have to work out a way to deal with it. Typically, you cannot ask your manager to give the whole team a week off so they can take out the trash. So in essence,you will have to find a way to partially eliminate these classes, one at a time. The most important part about this is to decide - with the whole team - what an Employee
really is. Do not integrate all of the individual attributes into one god-employee. Think more of a core-employee and most importantly, decide where that Employee
class will reside.
If you do code reviews, then it is particularly easy to ensure that the problem does at least not grow any further, i.e. stop everyone right in their track when they want to add another Employee
again. Also take care that new subsystems adhere to the agreed Employee
and are not allowed to access the old versions.
Depending on your programming language, you may also want to mark the classes that will eventually be eliminated with something like @Deprecated
to aid your team in immediately realizing that they're working with something that will have to be changed.
As for getting rid of the outdated employee classes, you can decide for each individual case how it's best eliminated, or just agree on a common pattern. You can affix the class name and wrap it around the real employee, you can use patterns (decorator or adapter come to mind), or, or or.
To make a long story short: This "practice" is technically sound, but is choked full of hidden costs that will only occur further down the road. While you may not be able to immediately get rid of the problem, you can start immediately with containing its harmful effects.
FooAdapter
,FooViewer
,FooCalculator
, etc. but certainly not named the same thing. Without more information about the specific classes about which you are talking it is difficult to provide an answer, however.Employee
as the example. There is an employee that is for other employees, one for HR, one for outside exposure. They're all named "employee" and they all have varieties of helpers (EmployeeRepository, EmployeeView, etc.). They all are in the same dll, and while they share some common properties (name, title), they all differ (phone number, salary).Employee
s several times, sounds like you need a model with the union of all the attributes and include atype
ordepartment
attribute. Otherwise, there is needless duplication of code.