tags:

views:

573

answers:

22
+3  A: 

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

Tim Mooney
Less prone to error, but probably more prone to code duplication.
jrudolph
+8  A: 

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.

Stephan
Maybe not always be flexibile if there are multiple fields of the same data type.getEmployeeName(string email)getEmployeeName(string passportNumber)getEmployeeName(string telephoneNumber)....
icelava
Many. More readable. If you need all of these variants, write all of them.
Gishu
That's not a good approach since you then can't have to criteria with the same type.
jrudolph
I think it is the good approach. If there is the problem with particular type being used for two different things, it actually points out a flaw in the design. emailId should probably belong to a structure/class not string, so is the case with telephone number, etc.
ragu.pattabi
ragu is right about that: If "email" and "passportNumber" are both of the same type, then perhaps this is the occasion to give them a real type. +1.
paercebal
Then take first name and last name, you probably wouldn't introduce a type for each of them, would you? It is only a coincidence that types can be used to distinguish between the roles of the properties of an object.
jrudolph
A: 

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.

Raithlin
+1  A: 

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

aaronjensen
You probably will have 4 almost similar methods. That's more to maintain not less...
jrudolph
Your definition of maintainability is not the same as mine.
aaronjensen
A: 

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.

larsivi
A: 

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.

micahwittman
+1  A: 

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

A: 

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.

snomag
A: 

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.

Javaxpert
Why rely on runtime exceptions when you can have static compile time safety?
jrudolph
I agree with jrudolph.What you are coding is pure old "C switch programming".While it is better than the byte array, this is not a good solution.
paercebal
A: 

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.

A: 

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.

FrankS
+1  A: 

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

m_pGladiator
But the line "getEmployeeName(Object SSN)" goes to far... The "Object" type would make almost any other overload meaningless (very much like the dread <code>void *</code> from C.
paercebal
+3  A: 

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

Sietse
A: 

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.

+7  A: 

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.

jrudolph
This is a great approach (I would personally use and have used it) but with one exception. Really consider beforehand whether you need all that extra code do something so simple. Sometimes simple is better.
Josh
Sounds like DSL to me.
ragu.pattabi
Overkill. The question is about accessors, which are already methods to access private fields. Some people would argue this is already too much overhead (and they would be wrong). What you propose is a full framework of filters... The user only wants the employee name... :-)
paercebal
Sure, you have to consider the amount of complexity such an approach seems to introduce. But I would prefer such an abstracted (and explicit) solution over one which duplicates code and is easily to be extended.
jrudolph
+1  A: 

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

timvw
A: 

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

paercebal
A: 

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.

PintSizedCat
+1  A: 

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.

adriaanp
A: 

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
}
Krakkos
This is a both nightmare maintenance, and the opposite philosophy of most current languages. Don't ever never ever never code that way.
paercebal
A: 

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?

Paralife
Because it is a good framework idea. But if the question is about accessors' naming convention, you don't want to instanciate 10 objects and make 10 comparisons, just to get some name. So the "jrudolf" solution can be considered overkill in most cases, and the **real right one** in some others.
paercebal
A: 

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.

moffdub