#74 java: more exception improvements

[phraktle]

Continuing #72, attached a patch with more Java API cleanup around exceptions. This is a relatively conservative step, still using checked exception (TJException) where appropriate. This could be made more specific, depending on what error conditions might make sense (e.g. ImageFormatException).

Note, that I have not updated the corresponding native code.

[DRC]

OK, this all makes sense except for the init()/destroy() stuff. The native code definitely needs to throw something there, because if it can't properly initialize or destroy the C API handle, then the library might be left in a bad state. What would be an appropriate unchecked error or exception to throw in that case?

[phraktle]

In what cases does init/destroy potentially fail? Mem allocation problem or something else too?

If they fail, what recourse would the invoker have to handle the problem?

(I would expect the right answer will be using an appropriate unchecked exception).

[DRC]

The problem is that the TurboJPEG API was designed to wrap a variety of different underlying codecs, so if a function in the underlying codec fails, TurboJPEG can't really make assumptions as to why. TJDecompressor.init() calls tjInitDecompress(), which calls jpeg_create_decompress(), and jpeg_create_decompress() can trigger any arbitrary error. That error is reported back only as a string.

Now, for the utility functions-- tjBufSize*(), tjPlane*(), tjGetScalingFactors()-- it is either explicitly specified in the API docs, or it can be implied, that all errors in those functions will be caused by bad arguments. So no problem-- I will always throw InvalidArgumentException in the JNI wrapper if those functions fail.

However, with tjInitCompress() and tjInitDecompress()-- I do think it's safe to say that if those functions fail, it's not a recoverable error, but there's also no way of saying that the functions will fail just because of a memory allocation error or just because of an argument error. The failure is reported from libjpeg or another underlying codec, so whatever failure message is reported by that codec needs to be passed through.

I agree-- let's use an appropriate unchecked exception for that. I just don't know which one is appropriate.

[phraktle]

Since the error in this case has no well-specified reason either, you could just throw a generic RuntimeException.

[DRC]

Here's the thing, though. The more I think about this, the more I think that a checked exception actually is correct for init() (maybe not for destroy().) For example, in TurboVNC (one of my other projects that is a consumer of libjpeg-turbo), I attempt to construct an instance of TJDecompressor, and if that fails, I fall back to using the built-in JPEG decompressor in the JVM. So in fact I do want to catch exceptions from the underlying library in that case.

[phraktle]

Your client code in TurboVNC would not need to change actually (since you already catch any Exception there, and RuntimeException is a subclass). The benefit is that you don't have to force all API users to deal with this.

[DRC]

OK, I think the best compromise is just to continue throwing a checked exception at the JNI level but to defer calling init() until within the body of the compress()/decompress()/transform() functions, which already throw checked exceptions.

Question: how should I credit the new code (TJException.java)? I can't just check it in without a license header, and I can't claim that I wrote it.

[DRC]

ARGH! No, not that simple, unfortunately. Because some of the constructors of TJTransformer and TJDecompressor call setSourceImage(), and setSourceImage() has to decompress the JPEG header, it also has to throw a checked exception. That means that those constructor signatures have to throw a checked exception.

This is fastly exceeding my threshold of pain, and I really have neither the time nor energy to contemplate how best to approach it. I think let's do the following:

-- Please provide me with an adequate copyright header for TJException.java.

-- I will make the necessary modifications so that, where possible, the Java methods don't throw checked exceptions at all, but the methods that have C functions underpinning them will throw TJException if those C functions have the potential to pass through errors from the underlying codec.

[phraktle]

Sorry about that, I was also hoping for an easy win :) (and of course Java should be sorry too for checked exceptions – all newer JVM-based languages go with unchecked-only)

Your approach sounds fine – in any case, it's a good improvement over the prior generic "throws Exception"

For the header: Copyright (C)2015 Viktor Szathmáry. (+ the usual)

[DRC]

git master branch now contains my latest & greatest version of this:

https://github.com/libjpeg-turbo/libjpeg-turbo

Please let me know if anything else is broken (you can reply here or create a new issue or pull request on GitHub if you prefer.)