Get Methods: One vs Many

Question

getEmployeeNameByBatchId(int batchID)
getEmployeeNameBySSN(Object SSN)
getEmployeeNameByEmailId(String emailID)
getEmployeeNameBySalaryAccount(SalaryAccount salaryAccount)

or

getEmployeeName(int typeOfIdentifier, byte[] identifier) -> In this methods the typeOfIdentifier tells if identifier is batchID/SSN/emailID/salaryAccount

Which one of the above is better way implement a get method?

These methods would be in a Servlet and calls would be made from an API which would be provided to the customers.

Answer 1

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.

Answer 2

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.

Answer 3

I'd go with the "many" approach. It seems more intuitive to me and less prone to error.

Answer 4

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).

Answer 5

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

Answer 6

First option, no question. Be explicit. It will greatly aid in maintainability and there's really no downside.

Answer 7

@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.

Answer 8

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.

Answer 9

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);

Answer 10

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.

Answer 11

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.

Answer 12

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.

Answer 13

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.

Answer 14

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.

Answer 15

I personally prefer to have the explicit naming "...ByRoomNumber" because if you end up with many "overloads" you will eventually introduce unwanted errors. Being explicit is imho the best way.

Answer 16

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 *".

Answer 17

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.

Answer 18

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?

Answer 19

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.

Answer 20

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 their own class type, I might have gone the second route, but switching internally on the class instead of a provided/application-defined type identifier.

Answer 21

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
}

Answer 22

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.