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

Performance of hashmap-based session object

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

Problem

Unfortunately, I can't use the Tomcat session for storing the key/value pairs for each user (restricted because it's an IVR domain-based project).

But I need the same functionality like a Tomcat session.

For every user I can get a unique id (call Id). So it's like the Tomcat sessionId.

I have developed the following class to achieve the Tomcat session functionality with my little knowledge of Java.

/** enum is used for Singleton instance  **/
public enum SessionObject {

    /** single instance  **/
    INSTANCE;

    /** ConcurrentHashMap to avoid external synchronization  **/
    /** Key as callId , value as HashMap to store the key value pairs  **/
    private ConcurrentHashMap> session =  new  ConcurrentHashMap>();

    /** Used to create a new Map when new call ivr is established in the first vxml file**/
    public void createSession(String callId){

        ConcurrentHashMap callMap = new ConcurrentHashMap();
        session.putIfAbsent(callId, callMap);
    }

    /** Used to get the existing stored values for the particular call Id **/
    public ConcurrentHashMap getSession(String callId){

        return session.get(callId);
    }

    /** Used to remove the details of the particular call Id , when call is disconnected **/
    public void deleteSession(String callId){

        session.remove(callId);
    }

}


I think that the code above will be more fine tunable. Suggestions to improve the performance would be appreciated.

Solution

This is pretty simple code, with not a lot of room for improvement.

You can make the session variable final, and it's good to do so:

private final ConcurrentHashMap> session = new ConcurrentHashMap>();


You can simplify createSession a bit by dropping the pointless local variable:

public void createSession(String callId) {
    session.putIfAbsent(callId, new ConcurrentHashMap());
}


Some of the comments are really pointless, for example these:

/** enum is used for Singleton instance  **/
public enum SessionObject {

    /** single instance  **/
    INSTANCE;


You're using spaces in a non-standard way. Instead of this:

private ConcurrentHashMap> session =  new  ConcurrentHashMap>();


Don't put a space in front of commas, but put one after, like this:

private ConcurrentHashMap> session = new ConcurrentHashMap>();


It's good that you comment your methods. But instead of this:

/** Used to create a new Map when new call ivr is established in the first vxml file**/
public void createSession(String callId){


The correct format for JavaDoc comments is like this:

/**
 * Used to create a new Map when new call ivr is established in the first vxml file
 * 
 * @param callId the call id for which to create new session
 */
public void createSession(String callId) {


Discussion

Our discussion in comments is perhaps worth including in the answer itself.


Is it necessary to make the method argument as final ?

In your code, it's not necessary. But it's recommended. By setting method parameters final, you can prevent reassigning the value by accident. It's a bad practice to reassign the values of method parameters, for example in here: void fun(int something) { something = 3; }


Can you please tell me, why you put the session variable as final ?

It's a good practice to make member fields final, as much as possible. final variables cannot change, they are immutable. Being immutable makes a variable more reliable and robust: once created correctly, you cannot break it, for example by accidentally reassigning to a value that wasn't correctly validated.


I have used the enum for singleton . So my class is thread safe. Also I declared a session variable as private . So, Is it necessary to use the concurrent hashmap instead of ordinary hashmap in this case ?

The enum singleton guarantees that all threads will see the same INSTANCE. However, if you have multiple threads, they might access the session map concurrently, which would not work with a regular hashmap. So yes, session should be a ConcurrentHashMap, unless the rest of your implementation guarantees single-threaded access.


By adding the final, the session variable become immutable. But I am adding the key/value pairs in the 'cmap'. So the map will become mutable.

I meant in the more general sense. For example simple types like int would be truly immutable. With object types, only their reference, the variable pointing to them will be immutable. Although their content is still editable (why you mustn't forget), not being able to reassign them does give you some level of protection. So it's always good to make your members final whenever possible. When your solution requires some members to be not final, give it some thought to refactor your solution to make them final. This is not always possible, but you should at least try, always.

Code Snippets

private final ConcurrentHashMap<String, ConcurrentHashMap<String, String>> session = new ConcurrentHashMap<String, ConcurrentHashMap<String, String>>();
public void createSession(String callId) {
    session.putIfAbsent(callId, new ConcurrentHashMap<String,String>());
}
/** enum is used for Singleton instance  **/
public enum SessionObject {

    /** single instance  **/
    INSTANCE;
private ConcurrentHashMap<String , ConcurrentHashMap<String,String>> session =  new  ConcurrentHashMap<String , ConcurrentHashMap<String,String>>();
private ConcurrentHashMap<String, ConcurrentHashMap<String, String>> session = new ConcurrentHashMap<String, ConcurrentHashMap<String, String>>();

Context

StackExchange Code Review Q#62648, answer score: 7

Revisions (0)

No revisions yet.