Your question about memory leaks
I want to start off by saying there will be no memory leak issues here. In fact, it would be exactly the opposite of memory leaks in both cases (they're the same in this respect). You can have only one instance of a String for the DataHolder class (since it is static, it does not belong to any DataHolder instance).
Imagine it was not static:
public class DataHolder
{
String dataString;
}
this means that for every time you do new DataHolder()
, you will have a separate instance of a String. With your static dataString, there will only ever be one instance.
Getters and Setters and Synchronization
The reason why your implementation is bad is because it is not thread safe, as Piyush Mittal pointed out. He didn't get into any details though, so I want to add my two cents.
You can't say that just because you only expect it to be used in a single thread that it will only be used in a single thread. The Swing thread is a great example of this, where there is only ever one thread that handles the UI. You would expect in this case that only one thread exists, and it is OK to do this. However, any background work must be done in a SwingWorker thread, so creating a SwingWorker thread here to do that will provide a data race opportunity because now there are two threads. You should always have getters and setters for static variables for this reason.
That being said, the code snippet you provided is exactly the same between your two examples (with the small difference of generating a new stack frame for the new function) because there is no synchronization. You want to do this:
public class DataHolder{
private static String dataString;
public static String getDataString(){
synchronized(DataHolder.class){
return DataHolder.dataString;
}
}
public static void setString(String dataString){
synchronized(DataHolder.class){
DataHolder.dataString = dataString;
}
}
}
I did change some things about your code. The first and most obvious is there is no this keyword used for static variables. As I hinted at before, because its static it does not belong to any instance. 'this' is not a thing in a static context.
The other thing I did was put the synchronized inside of the method. I did this for two reasons. The first was preference; some people prefer to keep synchronized out of the function signature so it is obfuscated from the caller, so they don't make assumptions because the function is synchronized. That is preference. The other reason I did this, is because I wanted to make it obvious that when you synchronize a function in the signature, this is how it actually will look 'under the hood'. As mentioned, since there is no 'this', it will actually synchronize on the class itself.
I think you could probably use some reading about the static keyword. I think your confusion is coming from the fact that you don't understand it, and you're trying to answer a question about getters and setters for the static variable before understanding statics themselves. Here is a link I found with a quick google search that might get you started:
http://www.javatpoint.com/static-keyword-in-java