Make the StreamGenerator an AutoCloseable

[ashleyfrieze]

This would allow StreamGenerator to be used with try-with-resources

[dghgit]

Thanks for the contribution, but I have to knock this one back, the file is package private so it would be of limited use, but also PGP is often used as a real protocol, so auto-closing the underlying stream could cause havoc and close operations are better handled by hand as suits the user of the API.

[ashleyfrieze]

also PGP is often used as a real protocol, so auto-closing the underlying stream could cause havoc and close operations are better handled by hand as suits the user of

@dghgit in practice, that close method is identical to the one in Autocloseable and while there are times when you wouldn't want to use it in a try-with-resources, the motivation for making this PR was this code:

PGPCompressedDataGenerator compressedDataGenerator = new PGPCompressedDataGenerator(PGPCompressedData.ZIP);
PGPLiteralDataGenerator literalDataGenerator = new PGPLiteralDataGenerator();
try {
   // our stuff
} finally {
  literalDataGenerator.close();
  compressedDataGenerator.close();
]

In other words, the home-made close method prevents us using try-with-resources when we want to, so we have to do it long-hand. I'm going to guess that this boilerplate is repeated thousands of times over people's codebases.

[dghgit]

That's not a bad point, but perhaps the trick here is to introduce a new class/interface which can act as a wrapper? Suggestions most welcome.

[ashleyfrieze]

That's not a bad point, but perhaps the trick here is to introduce a new class/interface which can act as a wrapper? Suggestions most welcome.

If you see how Closeable solved the problem, I continue to suggest my suggestion. This, I think, is a case of duck typing. It closes like a closeable, therefore it's probably a closeable.

The purpose and behaviour of close is identical to Closeable and has the same signature. While StreamGenerator isn't itself a good name for a closeable, I think we can say that stream generator IS A closeable thing.

I'm sorry to come back over and over with my original suggestion, rather than suggest new stuff BTW. Anything else I might think of would be more complex and a deeper change, where I just see this as StreamGenerator serves the function of Closeable and can join forces with it.

[ashleyfrieze]

@dghgit - to be fair, let's just look at what a wrapper might look like.

public class AutoCloseableWrapper<T extends StreamGenerator> implements Closeable {
    private final T wrapped;
    
    public AutoCloseableWrapper(T wrapped) { this.wrapped = wrapped };
    
    @Override
    public void close() throws IOException {
        wrapped.close();
    }
    
    public T get() {
       return wrapped;
    }
}

So in a try with resources you could go:

try (AutoCloseableWrapper<SomeThing> wrapped = new AutoCloseableWrapper<>(new Something()) {
   Something something = wrapped.get();
   // do the thing
}

The problem with this approach is the hassle of wrapping and unwrapping the thing, all for the trivial bridge to the close method.

[dghgit]

I'm not sure how this deals with the issue that StreamGenerator is not meant to close the underlying stream though?