CODEWITHSUNDEEP
javacollectionsdictionary
nobody
nobody

Reputation: 1949

Getting value from Map

I need to get one data from a collection (get() calls in the range of 100K for a single file processing).

public class DemoCollection {

    private Map<GroupCriteria, GroupData> collectionHolder = new ConcurrentHashMap<GroupCriteria, GroupData>();

    /**
     * 
     * @param groupCriteria
     *            GroupCriteria
     * @return GroupData
     */
    public GroupData getGroupForGroupingCriteriaOne(GroupCriteria groupCriteria) {
        GroupData groupData = null;
        if (collectionHolder.containsKey(groupCriteria)) {
            groupData = collectionHolder.get(groupCriteria);
        } else {
            // Get from database
        }
        return groupData;
    }

    /**
     * 
     * @param groupCriteria
     *            GroupCriteria
     * @return GroupData
     */
    public GroupData getGroupForGroupingCriteriaTwo(GroupCriteria groupCriteria) {
        GroupData groupData = null;
        if ((groupData = collectionHolder.get(groupCriteria)) == null) {
            // GEt from database
        }
        return groupData;
    }
}

Which is the best practice in this regard ? The approach one (getGroupForGroupingCriteriaOne) , two (getGroupForGroupingCriteriaTwo) or neither ?

Usually I ignore these premature optimization things, but since the get() calls are too huge I'm little bothered.

Could you please advice ?

Upvotes: 1

Views: 518

Answers (5)

Swapnil
Swapnil

Reputation: 897

In general I agree with the responses that getGroupForGroupingCriteriaTwo is better since it accesses the map only once, however your concern that since the map holds 100K items the access time will be high is misplaced.

You are using a ConcurrentHashMap, HashMap lookups have computation complexity of O(1) which means irrespective of the size of the data, these calls will return in constant time.

This optimization will not improve the performance. If you are really concerned about improving performance, consider using a proper caching framework.

Upvotes: 1

Paul Bellora
Paul Bellora

Reputation: 55213

Consider using Guava's MapMaker:

private ConcurrentMap<GroupCriteria, GroupData> collectionHolder = new MapMaker()
        .makeComputingMap(
           new Function<GroupCriteria, GroupData>() {
              @Override
              public GroupData apply(GroupCriteria key) {
                 //get from database and return
              }
        });

This ConcurrentMap will handle all concurrent requests for you. See the MapMaker documentation for a list of all configurable features of the produced map.

Upvotes: 1

Andrew White
Andrew White

Reputation: 53496

Version two is probably better for the reasons pointed out by others, but why not be less cryptic...

public GroupData getGroupForGroupingCriteriaThree(GroupCriteria groupCriteria) {
    GroupData groupData = collectionHolder.get(groupCriteria);

    return groupData != null ? groupData : callGetDataFromDB();
}

Upvotes: 1

Saket
Saket

Reputation: 46127

getGroupForGroupingCriteriaTwo looks perfectly reasonable. getGroupForGroupingCriteriaOne makes two lookups to the Map - one for searching the 'key' and the other for fetching the value.

However, I am hoping that after fetching from database, you would put the object into the Map (as a cache) so that the same can be used from the Map next time, rather than querying.

Upvotes: 1

RichW
RichW

Reputation: 2024

getGroupForGroupingCriteriaTwo is the way to go because you're asking the map to do the key lookup once instead of twice.

Upvotes: 2

Related Questions