It seems many places incorporate as best practice the use of class constants for SQL strings.
So instead of:
String sql = "select * from users";
PreparedStatement stmt = conn.prepareStatement(sql);
It's preferred to do:
private static final String SQL_SELECT_USERS = "select * from users";
void someMethod() {
...
PreparedStatement stmt = conn.prepareStatement(SQL_SELECT_USERS);
...
}
What are the advantages to the latter? It seems to me it's less readable.
Thanks.
If it is a short text and it is used only in one place then there is no need to make it a field. In your case it could be this
PreparedStatement stmt = conn.prepareStatement("select * from users");
You can find a lot of this kind of coding style in JDK source, like this one
if (is == null) {
throw new IllegalArgumentException("InputStream cannot be null");
}
There are really two issues at play here. The first is declaring the variable final; this provides a little bit of safety (you can't accidentally modify it later on, introducing a potentially intermittent runtime bug), and it indicates to anyone reading the code that the value is meant to be a constant.
Pulling the string out into a constant field (static final) is a bit more efficient and helps collect the important values in the class in one place. If the query statement is buried in someMethod(), it's harder to find and can't be reused between methods. By using class-level constants instead of "magic values", IDEs and Javadoc can identify their role, and developers needing to change the code later can find them more easily.
A final variable cannot be changed, by mistake or otherwise. Declaring the statement string as final makes sure to avoid this bug:
String s = "select * from users";
// many lines of code
s = "temp";
// do something with s
// many lines of code
PreparedStatement stmt = conn.prepareStatement(sql);
No matter how many lines of code you add after declaring s, if s is final the compiler will tell you when some code attempts to clobber it with a new value.
If you always do the prepared statement immediately after the variable, then simple proximity will help you avoid this error. Also, your actual example used a better name (sql rather than just s). But still, you never know who will edit your code after you write it.
Any time you can use a feature of the language to get the compiler to help you, you should do it. In this case, the final declaration lets the compiler protect you from someone clobbering your pre-defined string. Admittedly in this specific example the benefit seems pretty small, but in general constant things should be declared final and I don't see any reason to break that rule for this case.
As for declaring it private, that is just typical data hiding. If this string isn't part of the "interface" to this class, it should be private; by default, make everything private, and only make the interface stuff public.
EDIT: One more point worth considering. If you have a literal string that contains SQL, and you make some mistake when writing the SQL, the compiler cannot help you. "selct * from users" is a perfectly valid string; the Java compiler doesn't know it's an SQL error, so you find out at runtime.
You can make constants that are SQL fragments, and put them together with string concatenation. The great part about this is that if you misspell something, now you likely have a compiler error.
private final String SELECT_STAR_FROM = "select * from ";
private final String USERS_TABLE = "users";
// many lines of code
PreparedStatement stmt0 = conn.prepareStatement(SELECT_STAR_FROM + USERS_TABLE);
// this line would fail at run time
PreparedStatement stmt1 = conn.prepareStatement("selct * from users");
// this line fails at compile time and the compiler points you at it
PreparedStatement stmt0 = conn.prepareStatement(SELCT_STAR_FROM + USERS_TABLE);
When you do JNI programming you need to specify function signatures with cryptic codes. I made a bunch of constants and my JNI programs concatenate the constants together to build up the function signatures. This is C code, not Java, but it illustrates the same idea as the above.
#define JSIG_CONSTRUCTOR "<init>"
If I made a typo and wrote "<intt>" for a constructor, the C compiler couldn't help me; it would be a runtime error (JNI would fail to find the constructor). But if I use JSIG_CONSTRUCTOR in my code, if I make a typo, the compiler will tell me.
This is just one example of a very old (and very reasonable) programming practice of using constants instead of literals, e.g. the SQL_SELECT_USERS variable, instead of the "select * from users" literal string.
A similar approach is applicable (and advisable) for using numbers (e.g., BUFFER_SIZE instead of 4096) or any other type of value that is constant throughout the code of a class (or more than one classes). Constants (i.e., variables that are initialized only once and never change) in Java are declared using the final keyword.
Such is not a "less readable" strategy, either. On the contrary, if the name of the constant is intuitive, it makes much more sense than having the literal used in its place. Most importantly, after the constant is declared, it can be used as many times as necessary in the code and if, later on, its value needs to be changed (a very common case, indeed), the (whatever) change only needs to happen in one place (that of the declaration of the constant), instead of having to search through classes to make all the changes.
For constants that represent numbers, using the actual number instead of a single constant is called using a magic number in the code. That is not advisable because, after a while, it is difficult to remember why a value is defined as such. A constant with an intuitive name solves that problem.
So, using constants instead of literals is a strongly advisable programming habit to adopt.
The static final is no more "secure", nor is it faster, than simply coding the string literal at the point of use. (The number of strings would be the same, since literals are always interned, and the static final gets converted to a literal by the compiler anyway.)
The reason for using a more global value is to allow the string to be coded only once, vs being re-coded several places, and to collect all of the SQL string definitions in one location. This can be a good idea or a bad one, depending on circumstances (and the abilities of the programmer). One problem is that it can greatly obfuscate the code. Another is that if the value is defined in another class it must be class-prefixed, leading to some long names (If it's not defined globally like this then the advantage of coding only once is apt to be lost.)
It should be noted that it's foolishness to use this technique to "improve security", and then fail to use prepared statements for operations using externally-provided data values.
