Dienstag, 12. Mai 2009

Bad Habits - Static Variables

There are many bad habits programmers can develop in their life. The possible impact can range from hardly readable code to serious failures.

Today I want to discuss a very common habit that can cause very serious issues up to security and stability flaws: Static Variables.
No, I'm not speaking of constant values. It's ok to store, let's say, PI in a static constant. It's also ok to store environment properties and states in a static fashion, as they're affecting the whole system.

But you should be careful with data that is changing and/or context sensitive!

To clarify what I mean, lets assume someone with that habit is writing a parser that's parsing some data from a text. A possible worst case scenario could look like this:

class AParser
public static String data1;
public static String data2;

public static void parse(String text){
data1 = ...//parse data1
data2 = ...//parse data2
}
}

So what's wrong with it?
Imagine two threads calling parse at the same time. The result will be unpredictable, as both threads are working on the very same variables and are changing each others results.
It doesn't even need threads to go wrong badly! Every run will set values to some variables, but will it do that to every variable? Regardless of the circumstances? Most probably the answer is no. Even if it is, the next change of the method will likely add such problems.

So what's the solution? Simply removing static?
class AParser {
private String data1;
private String data2;

void parseName(String text){
data1 = ...//parse data1
data2 = ...//parse data2
}

public String getData1() {
return data1;
}
public String getData2() {
return data2;
}
}
Is that really better?
The answer is: No!
We just removed the static keyword, leaving the problem untouched: Multiple calls to the method of a single instance will still cause the same issues.
When used together with a lazy instantiation (singleton pattern) of the parser, there's no difference to the "real" static approach: there will be only a single instance to store data in. The variables would be static, even if they are not declared as such.

So what's the real problem here?
The major problem is the lack of a separation between data and logic, a common problem leading to static variables.
Another problem is the unnecessary wide scope of the variables.

So let's create a simple data class which holds the data locally and enables returning of multiple values:

class ParsedData {
String data1;
String data2;
}

class Parser {

public static ParsedData parse(String text){
ParsedData result = new ParsedData();
result.data1=...//Parse data1
result.data2=...//Parse data2
return result;
}
}
Now every call to the parse method will have it's own, local data. So it wont affect the data of other calls, even when the method itself is static.

Conclusion

  • Minimizing the the scope of your variables reduces possible side effects.
    Check every non-local variable if it is placed correctly
  • Introduce data classes for the data living in one context, to make handling with it easier and more flexible.

Keine Kommentare:

Kommentar veröffentlichen