1

Summary

I am looking at a scenario, such as this:

File someFile = null;
try
{
   someFile = File.createTempFile( SOME_PREFIX, SOME_SUFFIX, targetDirectory );
}
catch( IOException e )
{
    throw new SomeException( "Unable to create file for domain specific task", e, SomeExceptionErrorCode.FILE_MANAGEMENT_ERROR );
}
            
try( BufferedOutputStream stream = new BufferedOutputStream( new FileOutputStream( someFile.getAbsolutePath() ) ) )
{
    stream.write( byteData, 0, byteData.length );
    stream.flush();
}
catch( IOException e )
{
    throw new SomeException( "Unable to write domain specific data to domain specific file", e, SomeExceptionErrorCode.FILE_MANAGEMENT_ERROR );
}

For this scenario someFile is initialized with null. My intent is to translate this code into something that follows proper practices.

What I considered

  • Simply initializing someFile to null, as shown in the current code snippet. However typically I am avoiding this, so this does not seem satisfactory as of now
  • Initializing someFile with e.g. an empty String. This provides a default instance of File of sorts. The problem I see with this is, if this error handling changes in the future, a valid File with nonsense properties could be passed to some other place inside the code.
  • Nesting the try-catch blocks. This does work, however for some reason feels bad, especially since both nested blocks catch IOException
  • An Optional<File> was also considered, I am however not convinced if every try-catch block where a somewhat complex object is initialized to be used outside that block justifies the use of Optional

Question

Is initializing someFile to null an antipattern? If so, how is a scenario, such as the one posted, handled best?

Koenigsberg
  • 1,726
  • 1
  • 10
  • 22
  • You can simply omit the `= null` part. `File someFile;` is valid in this context. – luk2302 Mar 03 '21 at 16:57
  • Both answers suit my scenario nicely, thank you. Please note, technically speaking the question remains unanswered though. While the answers do help me to improve my code, the generalized question of *whether or not it is bad practice to initialize variables with `null`* has not yet been answered. – Koenigsberg Mar 03 '21 at 18:21
  • Initially I thought about placing the question without any context, but one-liners without context tend to attract negative attention if any, which is why I provided the explicit scenario I wanted to resolve. – Koenigsberg Mar 03 '21 at 18:22
  • Initializing with `null` is in general no bad practice. It's a fundamental part of the language. The alternative is: Wrapping **everything** in `Optional`s. I'd say that *this* is a bad practice, because it would make the code far more unreadable. And that's the most important part of code: It should be easy to understand! `...` Another thing to mention: Initializing with `null` and then overriding it would tell the compiler that this variable is **not** "effectively final". Though it cannot be used inside lambda expressions. Maybe (just a guess) it will also prevent compiler optimizations. – Benjamin M Mar 03 '21 at 18:55

2 Answers2

3

How about something like this:

public void yourMethod() {
  File file = createFile();
  writeFile(file);
}

private File createFile() {
  try {
    return File.createTempFile(...);
  } catch(...) {
    ...
  }
}

private void writeFile(File file) {
  try(...) {
    ...
  } catch(...) {
    ...
  }
}

So your method stays clean and easy to understand.

EDIT: Or even return an Optional<File> from createFile:

private Optional<File> createFile() {
  try {
    return Optional.of(File.createTempFile(...));
  } catch(...) {
    ...
    return Optional.empty();
  }
}

Then you can use Optional.ifPresent in yourMethod:

public void yourMethod() {
  Optional<File> file = createFile();
  file.ifPresent(value -> writeFile(value));

  // or shorter:
  createFile()
    .ifPresent(this::writeFile);

  // depends on how exactly the methods receive their parameters
}
Benjamin M
  • 23,599
  • 32
  • 121
  • 201
  • As written in the opening post, I thought about utilizing `Optional`, however I am wondering whether this introduces unnecessary complexity in a rather trivial task. However I do like the brevity introduced by passing a method reference to `Optional#ifPresent` when using subroutines. – Koenigsberg Mar 03 '21 at 18:16
  • 1
    Since Java 8 basically all of my "possibly null" methods return `Optional`, so there no method in my code that returns `null`. Every method either *always* returns some Object (then there's no need for Optional) or it returns an Optional. This way there are no more unexpected NullPointerExceptions caused by method return values. But when it comes to local variables (within a method) I basically never create Optionals, because it makes things complicated: Within a 10 lines long method I can easily see what's going on. But when calling a method I don't want to always look at its implementation. – Benjamin M Mar 03 '21 at 18:31
  • I can confirm that, but getting a bit cautions about overusing `Optional` by slamming it into each and every trivial scenario I happen to come accross. Last I checked usage of `Optional` outside the scope of its declared design goals was debated, but maybe things have changed. Personally I prefer using `Optional` even as members to `null` returns/assignments. – Koenigsberg Mar 03 '21 at 18:41
  • With regard to the overuse of `Optional` debate, I am referring to some of the responses and comments here, especially the linked material: https://stackoverflow.com/questions/23454952/uses-for-optional – Koenigsberg Mar 03 '21 at 18:42
  • I wouldn't say it's about "overuse". It's about readability and clarity: A 50 lines long method is hard to read and understand. Instead using 5 small helper methods can drastically improve this. And the same applies to `Optional`: It doesn't provide you anything new. You can write the same code logic using `null`. It depends on the concrete scenario if `Optional` brings benefits or makes things worse. For method return types I'd say: It makes things easier if you follow the rules to **always** return `Optional` if the return value could be `null`, because you can instantly see what's going on. – Benjamin M Mar 03 '21 at 19:10
  • 1
    @Koenigsberg The rule is simple: (A) If null is a legitimate value returned from a method, use `Optional`. That is not overuse, that is exactly why `Optional` was created (see writings by Brian Goetz). (B) If null is *not* a legitimate value to be returned from a method, then that method should call `Objects.requireNonNull` to throw an exception if a null is about to be returned. (C) In either case, the *calling* method should not be responsible for null checks, the *called* method is responsible for null checks (or wrapping in an `Optional`). Well, that’s the rule for code under your control. – Basil Bourque Mar 03 '21 at 19:31
2

You can just have

File someFile;

without any explicit assignment.

Java would then normally complain about using that variable before it has a value but the compiler is smart enough to understand that the only way the variable might not have a value is if createTempFile throws an IOException, but since you catch that and then throw again it knows that either the method exits here or someFile has a proper value. Therefore the later usages of someFile.getAbsolutePath() are allowed.

This is cleaner than null because now if you e.g. remove the re-throwing of the exception your code will not longer compile because now the compiler can no longer infer a value will always be assigned. If you init with null and remove the re-throw you will run into an NPE later on.

Optionals are not needed here because the compiler can in this case differentiate between non-initialized value and initialized value.

luk2302
  • 55,258
  • 23
  • 97
  • 137
  • That is some nice insight, thank you. I was aware of being able to declare the variable without any assignment, but unaware of the compiler being able to contextualize this scenario and recognize its validity. – Koenigsberg Mar 03 '21 at 18:12