1

I recently encountered a situation in a Java project where I wrote a block like this:

    String user = System.getProperty("user.home").toUpperCase();
    user = user.substring(user.lastIndexOf(File.separator) + 1);

    final String path1 = "C:\\first\\path";
    final String path2 = "C:\\second\\path";
    final String uniquePath = "C:\\users\\" + user + "\\unique\\path";

I'm just getting into practice of marking variables as final for style, but not sure how to handle this type of example. As I understand it, final is often used not just to prevent a value from being changed, but to indicate to the reader right off the bat that this variable is immutable. In this example, someone sees that path1, path2, and uniquePath are not going to change, and that fundamentally changes the way they think about those variables.

My concern is that, in the above example, the paths are clearly final, and user is clearly not. It's not too hard (I think) to work out that user probably isn't going to be manipulated, and the only reason it isn't also final is because it can't be gotten in one expression. However, if I saw this code, it would be really difficult for me to mentally group user with the paths. The coder obviously wanted to point out which variables were final, and they didn't mark user.

It seems then that something like this would make things more clear:

    String temp = System.getProperty("user.home").toUpperCase();
    temp = temp.substring(temp.lastIndexOf(File.separator) + 1);

    final String user = temp;

    final String path1 = "C:\\first\\path";
    final String path2 = "C:\\second\\path";
    final String uniquePath = "C:\\users\\" + user + "\\unique\\path";

But I'm not sure if this is worth the extra space and variable needed.

Is there a consensus on which of these approaches is a better practice? Is there a better option I've not considered?

  • 6
    Maybe you could extract the temp variable and the code it's using to a method and then do final String user = yourNewFunction()? This doesn't exactly solve your questions but definitely is a way to do this too. – Mibac Jul 13 '17 at 20:03
  • 5
    I find the idea of using final on everything that doesn't actually change deeply misguided. If it's not immediately obvious in a method what changes and what doesn't, then the method is too long to understand, and adding further verbiage doesn't help. Things would be different if constness was the default and Java had an explicit var keyword for non-constant things, but unfortunately it's far too late to turn things around on this point. – Kilian Foth Jul 13 '17 at 21:03
  • 2
    As more support for Kilian Foth, Eric Elliot feels similarly about const in JavaScript: it's "o.k.", but far better to write short functions where the reader can easily see that the variable doesn't change. Also, IMO, final should be used for variables that are very important that they don't change, not variables that are "by the way this doesn't change". e.g., in your code, if path1 changed after the creation of uniquePath, would anybody care? – user949300 Jul 14 '17 at 3:34
  • 2
    final doesn't mean immutable. It's very important to understand the distinction. Immutable means there is no determinable change in state after the instance has been created. final just prevents the variable from being reassigned. For those of us that learned const in C++, I'll admit such a concept would have been nice in Java (const modifier meant only other const methods could be called, and const methods could not change state of instance, so essentially forcing an instance to be immutable on the fly). – Neil Jul 14 '17 at 10:16
  • 1
    As an aside, please, please just use the environment variables and other options the OS or language gives you to get at user directories, don't just construct them and don't make your own subdirectories in there. That's what APPDATA and so are for. Codel like this is why I still have programs using folders on my tiny C: when I've relocated everything to another drive, plus it might break on international versions of windows… – StarWeaver Jul 14 '17 at 14:16
8

One solution to this is to instead extract the code into a method and then just call it. This way you won't need to have the temp variable in the main method's scope and it won't "pollute" it. Below is an example refactored method

final String user = getUser();

final String path1 = "C:\\first\\path";
final String path2 = "C:\\second\\path";
final String uniquePath = "C:\\users\\" + user + "\\unique\\path";

/* you could use a more descriptive name */
private String getUser() {
    String userHome = System.getProperty("user.home").toUpperCase();
    return userHome.substring(userHome.lastIndexOf(File.separator) + 1);
}
  • Either this or just employ the File class: final String uniquePath = new File(System.getProperty("user.home"), "unique/path").getAbsolutePath() – Timothy Truckle Jul 14 '17 at 11:12
2

My problem with you code is that is not system independent. You should always use UNC naming for paths (using / instead of \). Javas (N)IO classes with take care of the system specific conversion.

But there is more:

String user = System.getProperty("user.home").toUpperCase();

You convert the username to uppercase. This might not be a problem in your target environment if (and only if) all usernames are uppercase by default. But If you have a user Strauß the resulting string will be STRAUSS. Your code will fail in this case because even windows will not "retranslate" STRAUSS to Strauß.

Also: The location of the user home can be tweaked by a policy in the active directory. The default location you anticipate will work for 99,9% of your users, but for some it will not, because if the administrators moved the user homes, they most likely also restrict write access to C:\ so that you program will fail to create that folder...

So the question arises:

 System.getProperty("user.home")

already gives you the 100% correct path to users home folder. Why do you construct it yourself?

  • You're completely right about System.getProperty("user.home") and .toUpperCase(). I copied those two lines from an earlier program where I needed an uppercase username, and I forgot that they got the user home folder then stripped the name out. Shame on me for copy/pasting without thinking. I'm not too concerned about not being system independent though, since the nature of this program means it'll only be run on Windows machines, but I'll swap \\ for / anyway, thanks! – Lord Farquaad Jul 14 '17 at 18:08
2

You wrote:

String user = System.getProperty("user.home").toUpperCase();
user = user.substring(user.lastIndexOf(File.separator) + 1);

But the first expression is not "user", it's something else. Just write:

final String userHomeUpperCase = ...
final String user = ...

N.B. Timothy Truckle is right, this is terrible Java code because it will only ever work on Windows. The Java library has methods to accomplish your goals without hard-coding Windows paths.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.