Uwe, are you suggesting this to the Tika team? How will it handle backward compatibility (you can't require everyone to use Java 7, can you?) Can I do it myself in my version of the code? Java 7 has other problems compiling Tika, as I found out yesterday.
Thank you, Mark On Wed, Aug 31, 2011 at 7:35 AM, Uwe Schindler <[email protected]> wrote: > Hi, > > > I think that's a good approach in general (if you open it, you close it). > It does > > look like it's well documented, which is great. > > > > > The exception to this rule is the Tika.parseToString(InputStream, > > > Metadata) method. The reason for making an exception here is that the > > > Tika facade class was designed primarily for convenience and to > > > minimize the amount of code that the consumer of the API needs to > > > write. The proper resource management pattern in this case would have > > > been: > > > > > > InputStream stream = ...; > > > try { > > > return tika.parseToString(stream, ...); > > > } finally { > > > stream.close(); > > > } > > > > > > However, since in this specific case the client application hardly > > > ever needs to use the stream for anything else and since in pretty > > > much all cases the stream in question is constructed right when the > > > parseToString call is made, it makes more sense for the > > > parseToString() method to take care of closing the stream. The result > > > is that the above code can be reduced to: > > > > > > return tika.parseToString(..., ...); > > > > I can appreciate this motivation, ie "convenience trumps consistency", > here. > > > > Still this inconsistency can lead to confusion and to bugs, but since > it's > only one > > API that's "the exception" I think it's OK? > > > > But maybe we can beef up its javadocs a bit, saying "NOTE: unlike all > other APIs > > parsing from an InputStream, this API closes the incoming InputStream for > you > > for convenience" or something? > > For Java 7 you could simplify this as a try-with-resources statement (which > does something similar as Lucene's IOUtils.closeSafely method in a finally > block): > > try (InputStream stream = ...) { > return tika.parseToString(stream, ...); > } > > If you can use this type of statement (and Tika also consequently uses the > Closeable interface in its public APIs like Lucene does), this is really > nice to work with. > > Uwe > >
