[ 
https://issues.apache.org/jira/browse/IO-846?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17811688#comment-17811688
 ] 

Elliotte Rusty Harold edited comment on IO-846 at 1/28/24 8:41 PM:
-------------------------------------------------------------------

So without even looking at the docs as an experienced Java developer what I 
expect for XmlStreamReader, or any other reader, is

XmlStreamReader reader = new XmlStreamReader(in);

where in is an InputStream. Of course, in could also be a File, or perhaps a 
few other things like a byte array. Now let me look at the docs and see what I 
see. Yep, that works. 

XmlStreamReader reader = new XmlStreamReader(in);

If I were actually writing code I wouldn't have had to look at the docs. There 
are some extra constructors to specify leniency and default encoding, though I 
suspect 9 times out of 10 I wouldn't need these. However they wouldn't get in 
my way.

As an experienced Java dev I do not expect a builder. I know what a builder is, 
and I can figure it out, but probably not without the docs. For instance is it 
an inner class? an outer class? Can I construct it or is there some sort of 
factory method somewhere? If there is a factory method is it called create or 
newInstance or newBuilder or something else? I've seen all of these, and 
there's no reliable convention I can use to guide me here. And looking at the 
docs it appears all of those are wrong. Apparently I'm supposed to do 
XmlStreamReader.setInputStream(in).build(). 

It is harder to figure out a builder than a  constructor. I know the name of 
the constructor. I don't know the name or a lot of other details about the 
Builder. I can just type the right signature for a constructor. The builder 
makes me stop and think about it. The info I need isn't even on one page or in 
one class. I have to jump between a lot of superclasses to find the methods i 
need. How this is supposed to be better than  a one liner I can type in my 
sleep, I don't see.

Of course, there are other methods there. Apparently I can also do:


XmlStreamReader reader = XmlStreamReader.setCharacterSequence("some data 
here").get().

Except I really shouldn't be able to do that because the whole point of 
XmlStreamReader is to convert bytes into chars, not to start with chars. The 
superclass hierarchy here doesn't really fit the problem. 

There are other ways it doesn't fit. What if I do this:

XmlStreamReader reader = XmlStreamReader.setCharacterSequence("some data 
here").setPath("/foo/file.txt").get().

What happens now? I have no idea. Does the character sequence win? The path? Is 
one concatenated to the next? Is an exception thrown? If so which exception by 
which method? With constructors, I can't even say this. 

This is all needlessly complex. Throw away the builders. Throw away the 
superclasses. Use boring, ordinary constructors like everyone's
 been using since 1995; and the library becomes so much easier to understand, 
use, and maintain.














was (Author: elharo):
So without even looking at the docs as an experienced Java developer what I 
expect for XmlStreamReader, or any other reader, is

XmlStreamReader reader = new XmlStreamReader(in);

where in is an InputStream. Of course, i could also be a File, or perhaps a few 
other things like a ByteSequence. Now let me look at the docs and see what I 
see. Yep, that works. 

XmlStreamReader reader = new XmlStreamReader(in);

If I were actually writing code I wouldn't have had to look at the docs. There 
are some extra constructors to specify leniency and default encoding, though I 
suspect 9 times out of 10 I wouldn't need these. However they wouldn't get in 
my way.

As an experienced Java dev I do not expect a builder. I know what a builder is, 
and I can figure it out, but probably not without the docs. For instance is it 
an inner class? an outer class? Can I construct it or is there some sort of 
factory method somewhere? If there is a factory method is it called create or 
newInstance or newBuilder or something else? I've seen all of these, and 
there's no reliable convention I can use to guide me here. And looking at the 
docs it appears all of those are wrong. Apparently I'm supposed to do 
XmlStreamReader.setInputStream(in).build(). 

It is harder to figure out a builder than a  constructor. I know the name of 
the constructor. I don't know the name or a lot of other details about the 
Builder. I can just type the right signature for a constructor. The builder 
makes me stop and think about it. The info I need isn't even on one page or in 
one class. I have to jump between a lot of superclasses to find the methods i 
need. How this is supposed to be better than  a one liner I can type in my 
sleep, I don't see.

Of course, there are other methods there. Apparently I can also do:


XmlStreamReader reader = XmlStreamReader.setCharacterSequence("some data 
here").get().

Except I really shouldn't be able to do that because the whole point of 
XmlStreamReader is to convert bytes into chars, not to start with chars. The 
superclass hierarchy here doesn't really fit the problem. 

There are other ways it doesn't fit. What if I do this:

XmlStreamReader reader = XmlStreamReader.setCharacterSequence("some data 
here").setPath("/foo/file.txt").get().

What happens now? I have no idea. Does the character sequence win? The path? Is 
one concatenated to the next? Is an exception thrown? If so which exception by 
which method? With constructors, I can't even say this. 

This is all needlessly complex. Throw away the builders. Throw away the 
superclasses. Use boring, ordinary constructors like everyone's
 been using since 1995; and the library becomes so much easier to understand, 
use, and maintain.













> Undeprecate RandomAccessFileInputStream constructors, deprecate the builder
> ---------------------------------------------------------------------------
>
>                 Key: IO-846
>                 URL: https://issues.apache.org/jira/browse/IO-846
>             Project: Commons IO
>          Issue Type: Bug
>          Components: Streams/Writers
>            Reporter: Elliotte Rusty Harold
>            Priority: Major
>
> The Builder pattern is actively negative here. It introduces at least one 
> runtime failure mode (not setting the underlying file) that is a compile time 
> error with a constructor. The constructors are simpler, more obvious, and 
> less error prone. 
> The builder pattern is **not** preferred to constructors in all cases. It is 
> a non-standard idiom outside the Java language that is appropriate when:
> 1. There are a large number of arguments.
> 2. At least two of the arguments have the same type so there is a real risk 
> of swapping them. 
> 3. Any combination of arguments is allowed.
> All three of those conditions are false in this case. 



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to