At the outset, I am aware that, this question has a lot of answers on stack overflow, but the more I read, the more confused I am, as to what exactly qualifies for a static method.
Here are some usecases I feel where static methods are applicable and should be used.
I see, the major argument around not using static functions is that, people consider them as a death to unit testing. However, for the above cases, It makes perfect sense to make the methods static right ?
I want to understand, why is there a notion that functions shouldn't be made static if they just depend on the argument passed to the function (from concurrent request perspective, this isn't a problem, since the functions are executed on different threads)
Section NA
The guides you have read have confused you. The problem isn't static.
Because they oversimplified. As in, removed a little too much. The actual problem isn't static, the keyword. The problem is static state. Especially mutable static state.
Those 2 words (mutable, and state) are important.
Take as a trivial example everybody knows about, string's toLowerCase() method. It's not static, right? We call someString.toLowerCase() and that expression resolves to a new string with every character in it lowercased.
Imagine I find this not quite suitable to the task; I want a .toTitleCase() method which lowercases every character in it, except for any character that follows any whitespace/the start of the string, we want to .toTitleCase() that character.
I could write it, it wouldn't be hard, but as I can't just edit the core JDK, it'd have to be:
public static String toTitleCase(String in) {
boolean tc = true;
char[] out = new char[in.length()];
for (int i = 0; i < in.length(); i++) {
char c = in.charAt(i);
out[i] = tc ? Character.toTitleCase(c) : Character.toLowerCase(c);
tc = Character.isWhitespace(c);
}
return new String(out);
}
So is it 'bad' because I just made a static thing?
No! - while that method is static, it has no state - invoking this method has zero effect on anything. It doesn't change how future invocations of toTitleCase operate, there is no problem invoking toTitleCase simultaneously from multiple threads, and so on. Generally 'state' requires fields - not necessarily static ones (if I have a singleton, then the instance fields of that are global state, as in, there's only one field, for the entire JVM instance) - this method doesn't read or write any fields other than the fields on objects provided to it (the in parameter) and things it creates per execution (the char[] out array) and thus trivially has no global state and thus is not what these guides saying static is bad for testing are talking about whatsoever.
That makes sense: The difference between this toTitleCase() method and string's own toLowerCase() method are programming language implementation details only (java has no extension method system; if it had the differences would be even tinier). It'd be weird if one is fine and the other is some egregious violation of a programming development guide.
Let's say you want to build a system that maps a US state's short code (such as CA) to its generally recognized full name (such as California). This would require some state - a map that maps keys (CA) to names (California). Clearly then it's [A] state, and [B] static ('global'). However, we can design it so that it is not mutable.
This is horrible in the sense of the guides you read:
public class StateNames {
private static final Map<String, String> names = new HashMap<>();
static {
names.put("CA", "California");
// 50 more
}
public static void setStateName(String code, String name) {
names.put(code, name);
}
public static String getStateName(String code) {
return names.getOrDefault(code, "Unknown State");
}
public static Collection<String> listStateCodes() {
return names.keySet();
}
}
Because now we have made mutable global state - the .setStateName call has an effect on all future calls from anywhere in the entire JVM (or at least that classloader context, by default JVMs have just the one classloader context for the entire app, hence, global), there are timing issues to think about and it's not threadsafe (what happens if 2 threads simultaneously attempt to set some state names?) - a big mess.
We can fix it by just eliminating the setStateName method entirely and decree that this class starts off pre-loaded with 50 state names and that's that, it cannot change for the life of the app cycle. If some state does something bizarre and secedes or wants to split up, you'd have to write an update to the app with a new list of states, roll it out, and ask everybody to restart the application.
However, to make that easy for yourself, you decide not to just write 50 .put() calls in the code, but to make a separate text file that lists each state and its code, and that this class will simply read it during static initialization, something like:
public class StateNames {
private final Map<String, String> STATE_NAMES;
public StateNames(InputStream resource) throws IOException {
var map = new LinkedHashMap<String, String>();
try {
try (
var raw = StateNames.class.getResourceAsStream("us_states.txt"));
BufferedReader br = new BufferedReader(raw);
) {
String line;
while (true) {
line = br.readLine();
if (line == null) break;
String[] parts = line.trim().split("\t", 2);
String stateCode = line[0];
String stateName = line[1];
map.put(stateCode, stateName);
}
}
} catch (IOException e) {
throw new RuntimeException("App jar corrupted - no state.txt", e);
}
STATE_NAMES = Collections.unmodifiableMap(map);
}
public static String getStateName(String code) {
return STATE_NAMES.getOrDefault(code, "Unknown State");
}
public static Collection<String> listStateCodes() {
return STATE_NAMES.keySet();
}
}
This is a lot better (for example, our earlier version had a subtle bug even if we remove the setStateName method: You could listStateCodes().iterator() and then call .remove() on the iterator which would remove states, whoops!)
From the perspective of any code that isn't this code, you don't have any issues with testing (you don't need to worry about 'what happens if some other code changes state codes halfway through this process'). But, we still have 2 problems, this isn't all that great for testing:
For these reasons its better to have a totally non-global StateNames implementation:
public class StateNames {
private final Map<String, String> stateNames;
public StateNames(InputStream resource) throws IOException {
try (BufferedReader br = new BufferedReader(raw)) {
var map = new LinkedHashMap<String, String>();
String line;
while (true) {
line = br.readLine();
if (line == null) break;
String[] parts = line.trim().split("\t", 2);
String stateCode = line[0];
String stateName = line[1];
map.put(stateCode, stateName);
}
}
stateNames = Collections.unmodifiableMap(map);
}
public String getStateName(String code) {
return stateNames.getOrDefault(code, "Unknown State");
}
public Collection<String> listStateCodes() {
return stateNames.keySet();
}
}
This code is actually testable! We can make a simple little text file with just 3 test states in a string literal, create a ByteArrayInputStream from that, create a new instance of StateNames from that, and check that it properly 'parses' our text file.
We can still have the concept of 'look up state name' be available globally simply by having a static singleton. Add to your StateNames class:
private static final StateNames INSTANCE; static {
try {
try (var in = StateNames.class.getResourceAsStream("us_states.txt")) {
INSTANCE = new StateNames(in);
}
} catch (IOException e) {
throw new RuntimeException("app corrupt - us_states.txt", e);
}
}
public static StateNames of() { return INSTANCE; }
This solves one half of our problems - our StateNames code is now testable. But, other code that uses StateNames, if we see a need to supply test-statename data to properly test them, still cannot be easily tested.
To solve that, you have 2 general options:
We could just pass an instance of StateNames to everything that needs it. That way, test code can make a test version and pass that, and for 'production' we can pass the actual full statenames, read from the us_states.txt file.
However, rigorous application of this idea tends to result in most classes in your application code ballooning to having 20 fields (it'd need a private final StateNames stateNames; now, after all), with constructors with tens of parameters which are quite unwieldy.
We've solved the 'hard to test' problem but replaced it with 'code is unwieldy and a small change requires updating lots of lines of code' problem; the cure was much worse than the disease, here!
Fortunately(?) there are dependency injection frameworks, such as guice, and spring also has its own I believe.
These take care of the job of constructing objects, by more or less being a repository that maps types (such as StateNames) to 'a strategy to obtain an instance of it', which a test run can configure as 'I will provide you with a test implementation', and a production run can configure as 'I will provide you with the real implementation'.
The other option is to add this frameworky business to your StateNames:
public class StateNames {
private static StateNames INSTANCE = null;
@ForTesting
public static void setTestData(StateNames test) {
INSTANCE = test;
}
public static StateNames of() {
if (INSTANCE == null) INSTANCE = createDefault();
return INSTANCE;
}
}
Where createDefault does the routine of reading out us_states.txt.
We've actually created a whole bunch of global mutable state here. And, a test run can really only meaningfully "pre-configure" test data (call a whole bunch of setTestData static methods on all classes like this), and only then start a test, and cannot modify any of this state in between because we then get into very hairy issues of caching and multi-core issues. You're the judge: If it's data you expect requires updating in the middle of tests in order to properly test your code, then you really have to go with the injection frameworks (or manually pass lots of arguments for every relevant constructor). But if not, even though this seems to 'violate the rule of mutable static state', it gives you what you want: The code itself is testable, and other code that uses this is testable.