I'd go with the "many" approach. It seems more intuitive to me and less prone to error.
Why not overload the getEmployeeName(??) method?
getEmployeeName(int BatchID)
getEmployeeName(object SSN)(bad idea)
getEmployeeName(String Email)
etc.
Seems a good 'many' approach to me.
I would use the first option, or overload it in this case, seeing as you have 4 different parameter signatures. However, being specific helps with understanding the code 3 months from now.
First option, no question. Be explicit. It will greatly aid in maintainability and there's really no downside.
The first is probably the best in Java, considering it is typesafe (unlike the other). Additionally, for "normal" types, the second solution seems to only provide cumbersome usage for the user. However, since you are using Object as the type for SSN (which has a semantic meaning beyond Object), you probably won't get away with that type of API.
All-in-all, in this particular case I would have used the approach with many getters. If all identifiers have theyr own class type, I may have gone the second route, but switching internally on the class insted of a provided/application defined type identifier.
Is the logic inside each of those methods largely the same?
If so, the single method with identifier parameter may make more sense (simple and reducing repeated code).
If the logic/procedures vary greatly between types, a method per type may be preferred.
@Stephan: it is difficult to overload a case like this (in general) because the parameter types might not be discriminative, e.g.,
- getEmployeeNameByBatchId(int batchId)
- getEmployeeNameByRoomNumber(int roomNumber)
See also the two methods getEmployeeNameBySSN, getEmployeeNameByEmailId in the original posting.
As others suggested the first option seems to be the good one. The second might make sense when you're writing a code, but when someone else comes along later on, it's harder to figure out how to use code. ( I know, you have comments and you can always dig deep into the code, but GetemployeeNameById is more self-explanatory)
Note: Btw, usage of Enums might be something to consider in some cases.
You are thinking C/C++.
Use objects instead of an identifier byte (or int).
My Bad, the overload approach is better and using the SSN as a primary key is not so good
public ??? getEmployeeName(Object obj){
if (obj instanceof Integer){
...
} else if (obj instanceof String){
...
} else if .... // and so on
} else throw SomeMeaningFullRuntimeException()
return employeeName
}
I think it is better to use Unchecked Exceptions to signaling incorrect input.
Document it so the customer knows what objects to expect. Or create your own wrappers. I prefer the first option.
In a trivial case like this, I would go with overloading. That is:
getEmployeeName( int batchID );
getEmployeeName( Object SSN );
etc.
Only in special cases would I specify the argument type in the method name, i.e. if the type of argument is difficult to determine, if there are several types of arguments tha has the same data type (batchId and employeeId, both int), or if the methods for retrieving the employee is radically different for each argument type.
I can't see why I'd ever use this
getEmployeeName(int typeOfIdentifier, byte[] identifier)
as it requires both callee and caller to cast the value based on typeOfIdentifier. Bad design.
If you rewrite the question you can end up asking:
"SELECT name FROM ... "
"SELECT SSN FROM ... "
"SELECT email FROM ... "
vs.
"SELECT * FROM ..."
And I guess the answer to this is easy and everyone knows it.
What happens if you change the Employee class? E.g.: You have to remove the email and add a new filter like department. With the second solution you have a huge risk of not noticing any errors if you just change the order of the int identifier "constants". With the first solution you will always notice if you are using the method in some long forgotten classes you would otherwise forget to modify to the new identifier.
The methods are perfect example for usage of overloading.
getEmployeeName(int batchID)
getEmployeeName(Object SSN)
getEmployeeName(String emailID)
getEmployeeName(SalaryAccount salaryAccount)
If the methods have common processing inside, just write one more getEmplyeeNameImpl(...) and extract there the common code to avoid duplication
I don't like getXByY() - that might be cool in PHP, but I just don't like it in Java (ymmv).
I'd go with overloading, unless you have properties of the same datatype. In that case, I'd do something similar to your second option, but instead of using ints, I'd use an Enum for type safety and clarity. And instead of byte[], I'd use Object (because of autoboxing, this also works for primitives).
You could use something like that:
interface Employee{
public String getName();
int getBatchId();
}
interface Filter{
boolean matches(Employee e);
}
public Filter byName(final String name){
return new Filter(){
public boolean matches(Employee e) {
return e.getName().equals(name);
}
};
}
public Filter byBatchId(final int id){
return new Filter(){
public boolean matches(Employee e) {
return e.getBatchId() == id;
}
};
}
public Employee findEmployee(Filter sel){
List<Employee> allEmployees = null;
for (Employee e:allEmployees)
if (sel.matches(e))
return e;
return null;
}
public void usage(){
findEmployee(byName("Gustav"));
findEmployee(byBatchId(5));
}
If you do the filtering by an SQL query you would use the Filter
interface to compose a WHERE clause.
The good thing with this approach is that you can combine two filters easily with:
public Filter and(final Filter f1,final Filter f2){
return new Filter(){
public boolean matches(Employee e) {
return f1.matches(e) && f2.matches(e);
}
};
}
and use it like that:
findEmployee(and(byName("Gustav"),byBatchId(5)));
What you get is similar to the Criteria
API in Hibernate.
Sometimes it can be more conveniant to use the specification pattern.
Eg: GetEmployee(ISpecification<Employee> specification)
And then start defining your specifications...
NameSpecification : ISpecification<Employee>
{
private string name;
public NameSpecification(string name) { this.name = name; }
public bool IsSatisFiedBy(Employee employee) { return employee.Name == this.name; }
}
NameSpecification spec = new NameSpecification("Tim");
Employee tim = MyService.GetEmployee(spec);
I agree with Stephan: One task, one method name, even if you can do it multiple ways. Method overloading feature was provided exactly for your case.
- getEmployeeName(int BatchID)
- getEmployeeName(String Email)
- etc.
And avoid your second solution at all cost. It smells like "thy olde void * of C". Likewise, passing a Java "Object" is almost as poor style as a C "void *".
If you have a good design you should be able to determine if you can use the overloading approach or if you're going to run into a problem where if you overload you're going to end up having two methods with the same parameter type.
Overloading seems like the best way initially, but if you end up not being able to add a method in future and messing things up with naming it's going to be a hassle.
Personally I'd for for the approach of a unique name per method, that way you don't run into problems later with trying to overload the same parameter Object methods. Also, if someone extended your class in the future and implemented another void getEmployeeName(String name) it wouldn't override yours.
To summarise, go with a unique method name for each method, overloading can only cause problems in the long run.
I will use explicit method names. Everyone that maintains that code and me later will understand what that method is doing without having to write xml comments.
stick all your options in an enum, the have something like the following
GetEmployeeName(Enum identifier)
{
switch (identifier)
case eBatchID:
{
// Do stuff
}
case eSSN:
{
}
case eEmailId:
{
}
case eSalary:
{
}
default:
{
// No match
return 0;
}
}
enum Identifier
{
eBatchID,
eSSN,
eEmailID,
eSalary
}
The decoupling between the search process and the search criteria jrudolf proposes in his example is excellent. I wonder why isnt it the most voted solution. Do i miss something?
I'd go with Query Objects. They work well for accessing tables directly. If you are confined to stored procedures, they lose some of their power, but you can still make it work.