commons-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jukka Zitting (JIRA)" <>
Subject [jira] Commented: (IO-192) Tagged input and output streams
Date Wed, 11 Feb 2009 12:38:59 GMT


Jukka Zitting commented on IO-192:

I like how you're extending the functionality to the Reader and Writer classes.

1) Its a useful feature to be able to handle exceptions - not just in this use-case for tagging,
but generally so IMO it would be good to move the exception handling into the Proxy stream
implementations. We could provide a protected handleException(IOException) method that by
default just re-throws the exception to keep compatibility, but a allows people to override
for their own custom exception handling.

Good idea.

My only issue is with the return value in the handleException() method of ProxyInputStream
and ProxyReader. For example the skip() method should never return -1 but there is no way
(apart from parsing the stack trace) for handleException() to know which method invoked it
and what return value would be appropriate. I'd rather have the handleException() method return
nothing, and just add fixed "return -1" or "return 0" statements where needed. A subclass
that needs to modify the return value based on a thrown exception should override the specific
method with custom processing.

2) Exceptions are Serializable and many stream implementations are not so I have some concern
about holding a reference to the stream in the TaggedIOException. Also this could cause references
to the stream being held longer than previously by the application and prevent/delay garbage
collection. An alternative could be to store the identity hash code of the tag object instead.

Good point, though I'm not so sure about using the identity hash for this. For most (all?)
JVMs it will be unique to the tag object (at least as long as the object lives), but there's
no guarantee that this actually is the case. Perhaps the tagged proxy classes should have
a "private final Object tag = new Object();" tag object for this purpose. This would make
the related IOUtils methods more complex, but see below for more on that.

3) The current solution requires users to reference the concrete tagged stream implementations.
While this is OK in your simple example within a single method its not good practice generally
and will either encourage people to pollute their API with these tagged streams or require
additional casting.

I don't see a use case where you'd need to use casts or pollute APIs with these classes.

I suggest we move the code for handling these streams into IOUtils - which also makes it more
generic and available to re-use for other tagging requirements, not just by the throwing stream.

I would rather put such static generic methods directly on the TaggedIOException class. This
would make it easier to reuse just that class.

In any case I would keep the current isCauseOf() and throwIfCauseOf() methods on the tagged
stream classes, as IMHO the instance method call is clearer than a static IOUtils (or TaggedIOException)
method call.

> Tagged input and output streams
> -------------------------------
>                 Key: IO-192
>                 URL:
>             Project: Commons IO
>          Issue Type: New Feature
>          Components: Streams/Writers
>            Reporter: Jukka Zitting
>            Assignee: Jukka Zitting
>            Priority: Minor
>             Fix For: 1.5
>         Attachments: IO-192-tagged-stream-changes.patch, IO-192.patch
> I'd like to introduce two new proxy streams, TaggedInputStream and TaggedOutputStream,
that tag all exceptions thrown by the proxied streams. The goal is to make it easier to detect
the source of an IOException when you're dealing with multiple different streams. For example:
> {code}
> InputStream input = ...;
> OutputStream output = ...;
> TaggedOutputStream proxy = new TaggedOutputStream(output);
> try {
>     IOUtils.copy(input, proxy);
> } catch (IOException e) {
>     if (proxy.isTagged(e)) {
>         // Could not write to the output stream
>         // Perhaps we can handle that error somehow (retry, cancel?)
>         e.initCause(); // gives the original exception from the proxied stream
>     } else {
>         // Could not read from the input stream, nothing we can do
>         throw e;
>     }
> }
> {code}
> I'm working on a patch to implement such a feature.

This message is automatically generated by JIRA.
You can reply to this email to add a comment to the issue online.

View raw message