commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sebb <seb...@gmail.com>
Subject Re: [io] How picky about backward compatibility ?
Date Sun, 21 Jun 2015 12:16:20 GMT
The Javadoc for the two constructors says:

>>
org.apache.commons.io.input.BrokenInputStream.BrokenInputStream(IOException
exception)

Creates a new stream that always throws the given exception.

Parameters:
exception the exception to be thrown
<<

and

>>
org.apache.commons.io.input.BrokenInputStream.BrokenInputStream()

Creates a new stream that always throws an IOException
<<

Similarly for the Output version of the class.

In the first case, it is clear that the same exception is thrown.
Though it does not explicitly say that the same *instance* is thrown.

The second case is less clear; it could be argued that any IOE would do.

So I agree that the original patch changed the behaviour in a way that
is contrary to the Javadoc and needed to be reverted.

I think it also broke the behaviour in another way - if the user
provided a sub-class of IOE, close() also ought to throw the same
subclass.

However, it does make sense for the classes to be usable with
try-with-resources.

One possible approach without breaking the behaviour would be to use a
separate close IOE field, and only populate it for the default
constructor.
However that is a bit of a cheat.

Another might be to add a new ctor which has a separate IOE parameter
for the close method.

Is there a genuine use-case which requires that the same IOE instance
be used? (Other than the unit tests!)
If not, then we could look into relaxing the behaviour (and fixing the
Javadoc) for a future release.
If there is a use-case for always using the same instance, then
providing an additional ctor would work.


On 20 June 2015 at 16:58, Gary Gregory <garydgregory@gmail.com> wrote:
> I'd like to look at the change but I get an "Invalid revision" when I click
> on the link.
>
> Do you have a diff or another link?
>
> Gary
>
> On Fri, Jun 19, 2015 at 10:48 PM, Kristian Rosenvold <krosenvold@apache.org>
> wrote:
>
>> I reverted the fix in
>> http://svn.apache.org/viewvc?view=revision&revision=r1686456 simply
>> because
>> it broke backward compatibility in a subtle way. The BrokenNNStream classes
>> supply the same instance of the IOException on all methods (and there are
>> tests that assert it)
>>
>> Given some of the discussions I've seen about backward compatibility here,
>> I felt it best to revert (even though API-wise I feel this change is within
>> reasonable even for a minor release). WDYT ?
>>
>> Kristian
>>
>
>
>
> --
> E-Mail: garydgregory@gmail.com | ggregory@apache.org
> Java Persistence with Hibernate, Second Edition
> <http://www.manning.com/bauer3/>
> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
> Spring Batch in Action <http://www.manning.com/templier/>
> Blog: http://garygregory.wordpress.com
> Home: http://garygregory.com/
> Tweet! http://twitter.com/GaryGregory

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Mime
View raw message