HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

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

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
thismethodmoreobjectwaybetterwritefinderorientedthere

Problem

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

Solution

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.

-
Some getObjects method, called like getObjects() instead of getTransactionTypes()

-
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  String nameByID(int id) {
    for (T obj : getObjects()) {
        if(obj.getId().equals(id)) {
            return obj.getName();
        }
    }
    return null;
}

Code Snippets

private static String nameById(Class whatClass, int id) {
    if(ProjectCode.class == whatClass) {
        // ...
    } else if(TransactionType.class == whatClass) {
        // ...
    }
    else if(CurrencyType.class == whatClass) {
        // ...
    }
    return null;
}
public static <T extends IInterfaceName> String nameByID(int id) {
    for (T obj : getObjects<T>()) {
        if(obj.getId().equals(id)) {
            return obj.getName();
        }
    }
    return null;
}

Context

StackExchange Code Review Q#7976, answer score: 2

Revisions (0)

No revisions yet.