aspose file tools*
The moose likes Java in General and the fly likes What could be potential problem with this code? Big Moose Saloon
  Search | Java FAQ | Recent Topics | Flagged Topics | Hot Topics | Zero Replies
Register / Login


Win a copy of Java 8 in Action this week in the Java 8 forum!
JavaRanch » Java Forums » Java » Java in General
Bookmark "What could be potential problem with this code? " Watch "What could be potential problem with this code? " New topic
Author

What could be potential problem with this code?

Ingudam Manoranjan
Ranch Hand

Joined: Jul 31, 2006
Posts: 48
public class AppMode
{
static String Mode;
public static boolean isTestMode()
{
Mode = "N";
/* get User Type from File or Cache */
String strUserType = PropertyManager.getProperty(Constants.USERTYPE_ID);
/* Get Mode from a Persistent Mode of storage or Cache ( if already loaded) */
Mode = PropertyManager.getProperty(Constants.MODE,strUserType);

/*Check if the Application is running in the Test mode*/
if(Mode.equals("Y"))
{
return true;
}
else
{
return false;
}
}
}
Lanny Gilbert
Ranch Hand

Joined: Jun 11, 2002
Posts: 103
PropertyManager.getProperty(Constants.MODE,strUserType);

could return NULL, so Mode would be NULL.

So, your check for Mode.equals("Y") could fail with a null pointer exception.
Barry Gaunt
Ranch Hand

Joined: Aug 03, 2002
Posts: 7729
For which objective of SCJP is your question meant for?
General Java coding problems are better put into one of our Java In General forums. Moving to Java In General (Intermediate).


Ask a Meaningful Question and HowToAskQuestionsOnJavaRanch
Getting someone to think and try something out is much more useful than just telling them the answer.
Ingudam Manoranjan
Ranch Hand

Joined: Jul 31, 2006
Posts: 48
I am looking more from multi threaded scenario.
Scott Johnson
Ranch Hand

Joined: Aug 24, 2005
Posts: 518
Mode is a static member. So isTestMode() has a race condition if called simultaneously by multiple threads. Each thread will be updating/reading the same string reference Mode which causes some threads to see the wrong value.

For example:

Thread 1 calls isTestMode()
Thread 1 sets Mode to 'N'.
Thread 1 sets Mode to 'Y' (value retrieved from PropertyManager)
Thread 2 calls isTestMode()
Thread 2 sets Mode to 'N'
Thread 1 reads Mode and sees 'N' but should see 'Y'

One solution is to synchronize the method isTestMode(). A better solution is to make the static member variable method local.
Ingudam Manoranjan
Ranch Hand

Joined: Jul 31, 2006
Posts: 48
Thanks Scott. I think the local option is the better one, as the JVM will take care of it, being a stack variable.

However the new challenge now is :
1) Since it is a stack variable, it will be recreated everytime and it will occupy some memory space. Concern:: It is used by a utility which is called many times.

So there are 2 options:
1) Make it local variable in isTestMode() method. Concern - above
2) Implement Singleton ( code given below) Concern - ?

Which one is better keeping the following factors in mind?
1) This code is run very frequently as a utility.


public class AppMode {
private static boolean Mode = false;
private static AppMode uniqueAppModeInstance = new AppMode();

/** Private Constructor to implement Singleton pattern */
private AppMode() {
String sTempMode = "N";
try {
/* get User Type from File or Cache */
String strUserType = PropertyManager.getProperty(Constants.USERTYPE_ID);
/* Get Mode from a Persistent Mode of storage or Cache ( if already loaded) */
sTempMode = PropertyManager.getProperty(Constants.MODE,strUserType);
} catch (Exception e) {
//..Do something
}
/*Check if the Application is running in the Test mode*/
if(sTempMode.equals("Y"))
{
Mode = true;
}
else
{
Mode = false;
}
}

public static boolean isTestMode()
{

if(uniqueApplicationModeInstance == null){
synchronized (AppMode.class)
{
if(uniqueAppModeInstance == null){
uniqueAppModeInstance = new AppMode();
}
}
}

return Mode;
}
}
[ August 08, 2006: Message edited by: Ingudam Manoranjan ]
Scott Johnson
Ranch Hand

Joined: Aug 24, 2005
Posts: 518
Since it is a stack variable, it will be recreated everytime and it will occupy some memory space.


Making it a local variable will not cause any performance issue. All you are creating on the stack is a reference to a String object.
Naseem Khan
Ranch Hand

Joined: Apr 25, 2005
Posts: 809
Hi Scott,

What I think is race condition can be solved even if Mode is declared as a static String, all you need to do is initialize with "N" at the time of declaration instead of explicitly assigning "N" in the method.

What you think?


Naseem
[ August 10, 2006: Message edited by: Naseem Khan ]

Asking Smart Questions FAQ - How To Put Your Code In Code Tags
Naseem Khan
Ranch Hand

Joined: Apr 25, 2005
Posts: 809
Well in that case also race condition will exist.

Following code will avoid the race condition if each thread will have a different instances of AppMode.




Naseem
Scott Johnson
Ranch Hand

Joined: Aug 24, 2005
Posts: 518
Following code will avoid the race condition if each thread will have a different instances of AppMode.


Yes, but what enforces each thread to have separate instances? What if the class is reused at a later time and the programmer doesn't know (or forgets) that the class has issues when used in a multithreaded application.

Good object oriented design says that variable scope should be as small as possible. (i.e., don't use a global variable when a local variable will work.) This minimizes the potential for bugs like this race condition.

In this particular example, Mode is not used outside of isTestMode() and no state is stored between invocations so Mode should be local.
Naseem Khan
Ranch Hand

Joined: Apr 25, 2005
Posts: 809
Good object oriented design says that variable scope should be as small as possible. (i.e., don't use a global variable when a local variable will work.) This minimizes the potential for bugs like this race condition.


Yes you are right.

Naseem
 
I agree. Here's the link: http://aspose.com/file-tools
 
subject: What could be potential problem with this code?
 
Similar Threads
weird behaviour when passing objects
Cache Implementaion
J Table Update (it makes me mad)
How to write a rescursive algorithm?
Making the Data Class a Singleton & Thread Safe