commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gary Gregory <garydgreg...@gmail.com>
Subject Re: [IO] Missing Reader / Writer implementations
Date Sat, 10 Aug 2019 00:56:54 GMT
I think I went through your PRs and we have a green build on Travis now.
Thank you for your contributions.

Gary

On Fri, Aug 2, 2019, 09:06 Gary Gregory <garydgregory@gmail.com> wrote:

> Thanks Rob, I should be able to review these over the weekend.
>
> Gary
>
> On Fri, Aug 2, 2019, 08:48 Rob Spoor <apache@icemanx.nl> wrote:
>
>> The PRs are done, and they build successfully (apart from Java 13 and
>> ea, but master has the same problem).
>>
>> I had to duplicate ClosedReader and ClosedWriter because of unit tests,
>> and also had to include BrokenReader and BrokenWriter because of unit
>> tests.
>>
>> Rob
>>
>>
>> On 01/08/2019 18:24, Rob Spoor wrote:
>> > Sure :)
>> >
>> > On 01/08/2019 18:03, Gary Gregory wrote:
>> >> May I suggest one PR per class so we can avoid a giant PR to review?
>> >>
>> >> Gary
>> >>
>> >> On Thu, Aug 1, 2019, 11:57 Rob Spoor <apache@icemanx.nl> wrote:
>> >>
>> >>> I guess that the full list is not necessary, but I've been working on
>> a
>> >>> project where I could have used the following:
>> >>>
>> >>> * TaggedReader: to distinguish between IOExceptions from reading and
>> >>> other IOExceptions.
>> >>> * TeeReader: a piece of code needs a Reader but I also need the
>> contents
>> >>> that have already been read. A TeeReader combined with a
>> >>> StringBuilderWriter would be perfect.
>> >>> * AppendableWriter: most of the code works with the more generic
>> >>> Appendable, but it sometimes needs to delegate to a library that only
>> >>> works with Writer.
>> >>>
>> >>> In addition, I think that the CloseShield classes are definitely a
>> good
>> >>> addition, especially CloseShieldWriter. Like CloseShieldInputStream,
>> >>> it's great for supporting wrapping Writer implementations that need
to
>> >>> be closed. These then need the Closed classes, to keep the
>> >>> implementation similar to those of ClosedInputStream and
>> >>> ClosedOutputStream.
>> >>>
>> >>> So let's shorten the list to the following:
>> >>> * CloseShieldReader: I've seen too many libraries that close Readers
>> >>> passed to them (even Jackson does this by default!).
>> >>> * ClosedReader: needed by CloseShieldReader.
>> >>> * TaggedReader: to support distinguishing between IOExceptions from
>> >>> reading and others.
>> >>> * TeeReader: to support my read + copy to StringBuilder problem.
>> >>> * AppendableWriter: to support my bridge problem.
>> >>> * CloseShieldWriter: see ClosedShieldWriter
>> >>> * ClosedWriter: needed by CloseShieldWriter.
>> >>>
>> >>> Maybe we can add TaggedWriter and TeeWriter for completeness sake.
>> >>>
>> >>>
>> >>> If nobody disagrees with these 7 or 9 classes (from the original list
>> of
>> >>> 22), I can start working on a pull request. I'll also include the
>> >>> IOUtils.writer method, using Objects.requireNonNull (I copied the
>> >>> explicit null check from IOUtils.buffer).
>> >>>
>> >>>
>> >>> Rob
>> >>>
>> >>>
>> >>> On 01/08/2019 14:10, Gary Gregory wrote:
>> >>>> Hi Rob,
>> >>>>
>> >>>> I imagine that the classes you mention might be missing based on
the
>> >>> YAGNI
>> >>>> principle. I offer that you consider real use cases before plunging
>> >>>> into
>> >>>> implementation. We do not want to bloat this component just for
the
>> >>>> sake
>> >>> of
>> >>>> completeness.
>> >>>>
>> >>>> Don't forget to get as close as possible to 100% code coverage with
>> >>>> unit
>> >>>> tests for new code in your PRs ;-)
>> >>>>
>> >>>> Gary
>> >>>>
>> >>>> On Thu, Aug 1, 2019 at 6:39 AM Rob Spoor <apache@icemanx.nl>
wrote:
>> >>>>
>> >>>>> Hi,
>> >>>>>
>> >>>>> I noticed that a lot of InputStream / OutputStream implementations
>> do
>> >>>>> not have a matching Reader / Writer implementation. Some would
>> >>>>> really be
>> >>>>> useful.
>> >>>>>
>> >>>>> I've made an overview of classes that could be added:
>> >>>>> * AutoCloseReader
>> >>>>> * BrokenReader
>> >>>>> * CloseShieldReader
>> >>>>> * ClosedReader
>> >>>>> * CountingReader
>> >>>>> * DemuxReader
>> >>>>> * InfiniteCircularReader
>> >>>>> * ObservableReader
>> >>>>> * TaggedReader
>> >>>>> * TeeReader
>> >>>>> * UnixLineEndingReader
>> >>>>> * WindowsLineEndingReader
>> >>>>>
>> >>>>> * AppendableWriter
>> >>>>> * BrokenWriter
>> >>>>> * CloseShieldWriter
>> >>>>> * ClosedWriter
>> >>>>> * CountingWriter
>> >>>>> * DeferredFileWriter
>> >>>>> * DemuxWriter
>> >>>>> * TaggedWriter
>> >>>>> * TeeWriter
>> >>>>> * ThresholdingWriter
>> >>>>>
>> >>>>>
>> >>>>> I am willing to write the following (based on the InputStream
>> >>>>> implementations), if needed. I have signed the ICLA, have a
JIRA
>> >>>>> account
>> >>>>> (username Spoor) and a GitHub account (username robtimus):
>> >>>>>
>> >>>>> * AutoCloseReader
>> >>>>> * BrokenReader
>> >>>>> * CloseShieldReader
>> >>>>> * ClosedReader
>> >>>>> * CountingReader
>> >>>>> * TaggedReader
>> >>>>> * TeeReader
>> >>>>>
>> >>>>> * AppendableWriter
>> >>>>> * BrokenWriter
>> >>>>> * CloseShieldWriter
>> >>>>> * ClosedWriter
>> >>>>> * CountingWriter
>> >>>>> * TaggedWriter
>> >>>>> * TeeWriter
>> >>>>>
>> >>>>>
>> >>>>> In addition, if AppendableWriter is added, I think there should
be a
>> >>>>> method added to IOUtils, to prevent having to create an
>> >>>>> AppendableWriter
>> >>>>> for an object that's already a Writer:
>> >>>>>
>> >>>>>        public static Writer writer(final Appendable appendable)
{
>> >>>>>            if (appendable == null) {
>> >>>>>                throw new NullPointerException();
>> >>>>>            }
>> >>>>>            return appendable instanceof Writer ?
>> >>>>>                    (Writer) appendable : new
>> >>> AppendableWriter(appendable);
>> >>>>>        }
>> >>>>>
>> >>>>>
>> >>>>> Kind regards,
>> >>>>>
>> >>>>> Rob
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> For additional commands, e-mail: dev-help@commons.apache.org
>>
>>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message