CODEWITHSUNDEEP
javaoopjakarta-ee
kittugadu
kittugadu

Reputation: 173

Get Methods: One vs Many

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.

Upvotes: 10

Views: 873

Answers (22)

larsivi
larsivi

Reputation: 1505

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.

Upvotes: 0

Javaxpert
Javaxpert

Reputation: 2355

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.

Upvotes: -2

moffdub
moffdub

Reputation: 5314

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.

Upvotes: 0

Paralife
Paralife

Reputation: 6236

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?

Upvotes: 0

Krakkos
Krakkos

Reputation: 1479

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
}

Upvotes: -1

timvw
timvw

Reputation: 346

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

Upvotes: 1

adriaanp
adriaanp

Reputation: 2074

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.

Upvotes: 1

Henry B
Henry B

Reputation: 8047

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.

Upvotes: 0

paercebal
paercebal

Reputation: 83364

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

Upvotes: 0

Stephan
Stephan

Reputation: 749

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.

Upvotes: 9

jrudolph
jrudolph

Reputation: 8367

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.

Upvotes: 7

Philip Helger
Philip Helger

Reputation: 1864

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.

Upvotes: 0

Sietse
Sietse

Reputation: 7955

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

Upvotes: 3

m_pGladiator
m_pGladiator

Reputation: 8620

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

Upvotes: 2

FrankS
FrankS

Reputation: 2422

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.

Upvotes: 0

user9116
user9116

Reputation: 597

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.

Upvotes: 0

Adam Vigh
Adam Vigh

Reputation: 1250

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.

Upvotes: 0

Thomas
Thomas

Reputation:

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

Upvotes: 1

micahwittman
micahwittman

Reputation: 12476

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.

Upvotes: 0

Aaron Jensen
Aaron Jensen

Reputation: 6060

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

Upvotes: 1

Raithlin
Raithlin

Reputation: 1774

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.

Upvotes: 0

Tim Mooney
Tim Mooney

Reputation: 182

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

Upvotes: 3

Related Questions