3

I searched about best ways to hold/share data that are to be used globally and as expected I found a lot of answers. One of the ways that caught my attention is by using static fields. However, the static field way stated there was like this:

public class DataHolder
{
   private static String dataString;

   public static String getDataString
   {
      return dataString;
   }

   public static void setString(String dataString)
   {
      this.dataString = dataString;
   }
}

But I am always doing it this way:

public class DataHolder
{
   public static String dataString;
}

I am just wondering isn't the latter much more easier than the first? Because I don't have to set any getter and setter methods. And I don't see any difference between the two. So why not recommend the second one?

I was also told before that doing my way would result to memory leak problem sometimes. But won't the first solution lead to memory leak issues?

Piyush Mittal
  • 1,860
  • 1
  • 21
  • 39
JLT
  • 3,052
  • 9
  • 39
  • 86
  • `Static` methods rarely used for these attempts (`get` or `set`). – Andrew Tobilko Aug 08 '15 at 16:28
  • 1
    You're hiding the implementation details (like all encapsulation). This allows you to modify it without breaking code. If you wanted to store the data in a `Data` type, you could change the field variable, then parse the `String` to a `Data` in the `setString` method (I advise changing the name), and from `Data` to `String` in the `getString` method. – Vince Aug 08 '15 at 17:10
  • 1
    you may want to read this link that essentially talks about need to get/set methods in first place! http://stackoverflow.com/questions/1568091/why-use-getters-and-setters?lq=1 – AADProgramming Aug 08 '15 at 17:23
  • if this is not a public API that will be used by other teams, don't worry, naked field is fine. you can always refactor later. – ZhongYu Aug 08 '15 at 18:30

3 Answers3

1

don have much idea about memory leaks.

But: multiple parallel requests - multiple threads, you surely have a threading issue here - this is not thread-safe unless you take care of (e.g. use synchronized).

Piyush Mittal
  • 1,860
  • 1
  • 21
  • 39
1

I would much prefer the getter/setters, rather than a public field.

That sort of encapsulation allows you to modify the types of properties without breaking the code of those using it.

What if you decided to create a Data type, rather than storing it as a String? (for increased type safety).

With a public field, it's not possible. But using getter/setters, you can perform the parsing needed to allow this:

class SomeClass {
    private static Data data;

    public static void setData(String data) {
        data = new Data(data);
    }

    public static String getData() {
        return data.toString();
    }
}

class Data {
    private String data;

    public Data(String data) {
        this.data = data;
    }

    public String toString() {
        return data;
    }
}

It's not a memory leak problem, as it has nothing to do with allocating memory. There may be some inconsistency between threads when using that data, since it is not thread-safe, but any leaking will not be a result of using static, rather than not multithreading properly.

Rather than accessing something globally, you should pass the dependency to where it's needed. For example, a method that uses a global variable. Pass the value to it through the parameters. This is called dependency injection.

Vince
  • 14,470
  • 7
  • 39
  • 84
1

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

searchengine27
  • 1,449
  • 11
  • 26