Is there a better, more object oriented way to write this Finder method?

Question

I have several lists of objects which are stored in different locations, based on what they are and who needs them - the system, or the user, for example. There are times where I am only given an ID and need to find the name, type, or description, but I would like a very succinct, efficient way to do so. Since I'm also trying to improve the quality of my code, I want to ensure I'm doing things the best way possible.

Is there a better way to do this? I first was passing in the object and using instanceof, but I thought this was a better way.

/**
 * This method searches all known types / locations to find the object name based on ID.
 * @param whatClass The type of the object you are searching for
 * @param id ID of the object for which you are searching.
 * @return The string name or type. Returns null if no match is found.
 */
public static <T> String nameByID(Class<?> whatClass, int id) {
 if (whatClass == ProjectCode.class) {
 for (ProjectCode c : User.getInstance().getProjects()) {
 if (c.getProjCdId() == id) {
 return c.getProjNm();
 }
 }
 } else if (whatClass == TransactionType.class) {
 for (TransactionType type : Session.getInstance().getTransactionTypes()) {
 if (type.getTransactionTypeId() == id) {
 return type.getTransactionNm();
 }
 }
 } else if (whatClass == CurrencyType.class) {
 for (CurrencyType currencyType : Session.getInstance().getCurrencies()) {
 if (currencyType.getCurrencyTypeId() == id) {
 return currencyType.getCurrencyTypeNm();
 }
 }
 }
 // no matches found
 return null;
}

• Paul Martel: Off hand, I'd guess that instanceof may provide a more robust solution in the context of derived classes. Can you show the instanceof alternative and also give examples of how you would typically call the two different functions?

Answer 1

It just doesn't smell good. Maybe you should post more code. This kind of searches looks weird to me.

Anyway, the User class should have a getProjNameById(final int id) method, the Session class should have getTransactionNameByTransactionTypeId(final int id) and getCurrencyTypeNameByCurrencyTypeId(final int id) methods as well. It would result higher cohesion and easier maintenance.

Note that the Singleton pattern is an antipattern nowadays. (The Singleton Pattern)

• Rob: I agree this is a good approach, but I believe a generic approach results in a still more flexible and concise implementation of nameById()
• DwB: I almost agree. I'd say User should have a getProjectNameById(final int id) method and Session should have getTransactionNameByTransactionTypeId(final int id) and getCurrencyTypeNameByCurrencyTypeId(final int id) methods. In general, avoid abbreviations; it is likely that your abbreviations will be meaningless gibberish to the next person to work on that code.
• Cody: My first and original implementation DOES in fact have methods like this in the Session / User class, but I thought it would be better to have one method being called. Preferably, I wouldn't want to hard-code anything, but have everything determined dynamically..I'm just unsure how.
• palacsint: Thanks @DwB, I agree, I forgot this. I've updated the answer.

Answer 2

If you're going to hard-code conditions for each class you want to support, there isn't much point to using a generic type. You don't even reference T within the method body, so it isn't any different from:

private static String nameById(Class whatClass, int id) {
 if(ProjectCode.class == whatClass) {
 // ...
 } else if(TransactionType.class == whatClass) {
 // ...
 }
 else if(CurrencyType.class == whatClass) {
 // ...
 }
 return null;
}

To write this method so it's reusable with a generic type, you'll need to also abstract both your methods for obtaining a collection, and your getters for obtaining instances' name and Id.

e.g.

1. Some getObjects<T> method, called like getObjects<TransactionType>() instead of getTransactionTypes()

2. The ability to access object properties polymorphically, through some interface that requires each class to have getId() and getName() instead of separate methods such as getTransactionTypeId(), getProjCdId(), etc.

Then your code could be rewritten to take advantage of a generic type parameter:

public static <T extends IInterfaceName> String nameByID(int id) {
 for (T obj : getObjects<T>()) {
 if(obj.getId().equals(id)) {
 return obj.getName();
 }
 }
 return null;
}

Answer 3

Why would you ever combine all this functionality into one method? Each case does completely different things. The "name" that the method retrieves from the ID has completely different content based on what it's looking for. Having three methods with the following signatures makes much, much more sense to me:

public static String getProjectName(int id)
public static String getTransactionName(int id)
public static String getCurrencyName(int id)

If you really want to keep it all in one method for some reason, use an enum as a valid option selector.

public enum NameType {
 PROJECT, TRANSACTION, CURRENCY;
}
public static String getNameByID(NameType type, int id) {
 switch(type) {
 case PROJECT:
 for(ProjectCode code : User.getInstance().getProjects()) {
 if(code.getProjCdId == id) return code.getProjNm();
 }
 break;
 case TRANSACTION:
 for(TransactionType type : Session.getInstance().getTransactionTypes()) {
 if(type.getTransactionTypeId() == id) return type.getTransactionNm();
 }
 break;
 case CURRENCY:
 for(CurrencyType type : Session.getInstance().getCurrencies()) {
 if (type.getCurrencyTypeId() == id) return type.getCurrencyTypeNm();
 }
 break;
 }
 return null;
}

I don't see the use of generics at all here.